[clangd] Add support to rename Objective-C selectors #78872
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-C selector names
llvmbot
added
clang
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang Author: Alex Hoppen (ahoppen) ChangesThis adds support to rename Objective-C method declarations and calls to clangd. Patch is 54.98 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78872.diff 24 Files Affected:
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9b..5dfd12045b65738 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -844,14 +844,15 @@ void ClangdLSPServer::onWorkspaceSymbol(
}
void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
- Callback<std::optional<Range>> Reply) {
+ Callback<PrepareRenameResult> Reply) {
Server->prepareRename(
Params.textDocument.uri.file(), Params.position, /*NewName*/ std::nullopt,
Opts.Rename,
[Reply = std::move(Reply)](llvm::Expected<RenameResult> Result) mutable {
if (!Result)
return Reply(Result.takeError());
- return Reply(std::move(Result->Target));
+ PrepareRenameResult PrepareResult{Result->Target, Result->OldName};
+ return Reply(std::move(PrepareResult));
});
}
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 79579c22b788a9c..6a9f097d551ae6d 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -134,7 +134,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
void onWorkspaceSymbol(const WorkspaceSymbolParams &,
Callback<std::vector<SymbolInformation>>);
void onPrepareRename(const TextDocumentPositionParams &,
- Callback<std::optional<Range>>);
+ Callback<PrepareRenameResult>);
void onRename(const RenameParams &, Callback<WorkspaceEdit>);
void onHover(const TextDocumentPositionParams &,
Callback<std::optional<Hover>>);
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 6fb2641e8793db1..b04ebc7049c6619 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -578,8 +578,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
// prepareRename is latency-sensitive: we don't query the index, as we
// only need main-file references
auto Results =
- clangd::rename({Pos, NewName.value_or("__clangd_rename_placeholder"),
- InpAST->AST, File, /*FS=*/nullptr,
+ clangd::rename({Pos, NewName, InpAST->AST, File, /*FS=*/nullptr,
/*Index=*/nullptr, RenameOpts});
if (!Results) {
// LSP says to return null on failure, but that will result in a generic
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index a6370649f5ad1cf..0291e5d71d65c88 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -905,6 +905,10 @@ llvm::json::Value toJSON(const DocumentSymbol &S) {
return std::move(Result);
}
+llvm::json::Value toJSON(const PrepareRenameResult &R) {
+ return llvm::json::Object{{"range", R.range}, {"placeholder", R.placeholder}};
+}
+
llvm::json::Value toJSON(const WorkspaceEdit &WE) {
llvm::json::Object Result;
if (WE.changes) {
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index e88c804692568f5..274c7fe4391062b 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1004,6 +1004,17 @@ struct CodeActionParams {
};
bool fromJSON(const llvm::json::Value &, CodeActionParams &, llvm::json::Path);
+struct PrepareRenameResult {
+ /// The range of the string to rename.
+ Range range;
+ /// A placeholder text of the string content to be renamed.
+ ///
+ /// This is usueful to populate the rename field with an Objective-C selector
+ /// name (eg. `performAction:with:`) when renaming Objective-C methods.
+ std::string placeholder;
+};
+llvm::json::Value toJSON(const PrepareRenameResult &WE);
+
/// The edit should either provide changes or documentChanges. If the client
/// can handle versioned document edits and if documentChanges are present,
/// the latter are preferred over changes.
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 835038423fdf372..0081a69357b02f4 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -625,16 +625,16 @@ llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
return Identifiers;
}
-std::vector<Range> collectIdentifierRanges(llvm::StringRef Identifier,
- llvm::StringRef Content,
- const LangOptions &LangOpts) {
+std::vector<Range>
+collectIdentifierRanges(llvm::StringRef Identifier,
+ const syntax::UnexpandedTokenBuffer &Tokens) {
std::vector<Range> Ranges;
- lex(Content, LangOpts,
- [&](const syntax::Token &Tok, const SourceManager &SM) {
- if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
- return;
- Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
- });
+ const SourceManager &SM = Tokens.sourceManager();
+ for (const syntax::Token &Tok : Tokens.tokens()) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+ continue;
+ Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+ }
return Ranges;
}
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index a1bb44c17612021..f60683fc4f21c35 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -217,9 +217,9 @@ llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
const format::FormatStyle &Style);
/// Collects all ranges of the given identifier in the source code.
-std::vector<Range> collectIdentifierRanges(llvm::StringRef Identifier,
- llvm::StringRef Content,
- const LangOptions &LangOpts);
+std::vector<Range>
+collectIdentifierRanges(llvm::StringRef Identifier,
+ const syntax::UnexpandedTokenBuffer &Tokens);
/// Collects words from the source code.
/// Unlike collectIdentifiers:
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index bf838e53f2a21e7..24bd3c426b3e86b 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -173,9 +173,20 @@ std::optional<RelationKind> indexableRelation(const index::SymbolRelation &R) {
bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
- if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ bool PrefixComparison;
+ switch (NameKind) {
+ case DeclarationName::Identifier:
+ case DeclarationName::CXXConstructorName:
+ case DeclarationName::ObjCZeroArgSelector:
+ PrefixComparison = false;
+ break;
+ case DeclarationName::ObjCOneArgSelector:
+ case DeclarationName::ObjCMultiArgSelector:
+ PrefixComparison = true;
+ break;
+ default:
return false;
+ }
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
const auto &LO = AST.getLangOpts();
@@ -183,7 +194,17 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
- return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
+ std::string LexerSpelling = clang::Lexer::getSpelling(Tok, SM, LO);
+ if (PrefixComparison) {
+ // The lexer spelling at the source location is only the first label of an
+ // Objective-C selector, eg. if `StrName` is `performAction:with:`, then the
+ // token at the requested location is `performAction`. Re-building the
+ // entire selector from the lexer is too complicated here, so just perform
+ // a prefix comparison.
+ return StringRef(StrName).starts_with(LexerSpelling);
+ } else {
+ return StrName == LexerSpelling;
+ }
}
} // namespace
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 11f9e4627af7609..f639cbc9b4b1273 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -26,6 +26,8 @@
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolName.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
@@ -40,6 +42,8 @@ namespace clang {
namespace clangd {
namespace {
+using tooling::SymbolName;
+
std::optional<std::string> filePath(const SymbolLocation &Loc,
llvm::StringRef HintFilePath) {
if (!Loc)
@@ -517,22 +521,27 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+ const SymbolName &NewName) {
trace::Span Tracer("CheckName");
static constexpr trace::Metric InvalidNameMetric(
"rename_name_invalid", trace::Metric::Counter, "invalid_kind");
auto &ASTCtx = RenameDecl.getASTContext();
+ auto Identifier = NewName.getSinglePiece();
+ if (!Identifier) {
+ return std::nullopt;
+ }
std::optional<InvalidName> Result;
- if (isKeyword(NewName, ASTCtx.getLangOpts()))
- Result = InvalidName{InvalidName::Keywords, NewName.str()};
- else if (!mayBeValidIdentifier(NewName))
- Result = InvalidName{InvalidName::BadIdentifier, NewName.str()};
- else {
+ if (isKeyword(*Identifier, ASTCtx.getLangOpts())) {
+ Result = InvalidName{InvalidName::Keywords, *Identifier};
+ } else if (!mayBeValidIdentifier(*Identifier)) {
+ Result = InvalidName{InvalidName::BadIdentifier, *Identifier};
+ } else {
// Name conflict detection.
// Function conflicts are subtle (overloading), so ignore them.
if (RenameDecl.getKind() != Decl::Function &&
RenameDecl.getKind() != Decl::CXXMethod) {
- if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
+ if (auto *Conflict =
+ lookupSiblingWithName(ASTCtx, RenameDecl, *Identifier))
Result = InvalidName{
InvalidName::Conflict,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
@@ -546,7 +555,7 @@ std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
// AST-based rename, it renames all occurrences in the main file.
llvm::Expected<tooling::Replacements>
renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+ const SymbolName &OldName, const SymbolName &NewName) {
trace::Span Tracer("RenameWithinFile");
const SourceManager &SM = AST.getSourceManager();
@@ -569,9 +578,30 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
- if (auto Err = FilteredChanges.add(tooling::Replacement(
- SM, CharSourceRange::getTokenRange(RenameLoc), NewName)))
- return std::move(Err);
+ if (std::optional<std::string> Identifier = NewName.getSinglePiece()) {
+ tooling::Replacement NewReplacement(
+ SM, CharSourceRange::getTokenRange(RenameLoc), *Identifier);
+ if (auto Err = FilteredChanges.add(NewReplacement)) {
+ return std::move(Err);
+ }
+ continue;
+ }
+ SmallVector<SourceLocation> PieceLocations;
+ llvm::Error Error = findObjCSymbolSelectorPieces(
+ AST.getTokens().expandedTokens(), SM, RenameLoc, OldName,
+ tooling::ObjCSymbolSelectorKind::Unknown, PieceLocations);
+ if (Error) {
+ // Ignore the error. We simply skip over all selectors that didn't match.
+ consumeError(std::move(Error));
+ continue;
+ }
+ for (auto [Location, NewPiece] :
+ llvm::zip_equal(PieceLocations, NewName.getNamePieces())) {
+ tooling::Replacement NewReplacement(
+ SM, CharSourceRange::getTokenRange(Location), NewPiece);
+ if (auto Err = FilteredChanges.add(NewReplacement))
+ return std::move(Err);
+ }
}
return FilteredChanges;
}
@@ -664,8 +694,9 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
// there is no dirty buffer.
llvm::Expected<FileEdits>
renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
- llvm::StringRef NewName, const SymbolIndex &Index,
- size_t MaxLimitFiles, llvm::vfs::FileSystem &FS) {
+ SymbolName OldName, SymbolName NewName,
+ const SymbolIndex &Index, size_t MaxLimitFiles,
+ llvm::vfs::FileSystem &FS) {
trace::Span Tracer("RenameOutsideFile");
auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath,
Index, MaxLimitFiles);
@@ -683,10 +714,11 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
}
auto AffectedFileCode = (*ExpBuffer)->getBuffer();
- auto RenameRanges =
- adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(),
- std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts());
+ syntax::UnexpandedTokenBuffer Tokens(
+ AffectedFileCode, RenameDecl.getASTContext().getLangOpts());
+ std::optional<std::vector<Range>> RenameRanges =
+ adjustRenameRanges(Tokens, OldName.getNamePieces().front(),
+ std::move(FileAndOccurrences.second));
if (!RenameRanges) {
// Our heuristics fails to adjust rename ranges to the current state of
// the file, it is most likely the index is stale, so we give up the
@@ -695,8 +727,8 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
"(the index may be stale)",
FilePath);
}
- auto RenameEdit =
- buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName);
+ auto RenameEdit = buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges,
+ OldName, NewName, Tokens);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
RenameEdit.takeError());
@@ -779,12 +811,25 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
const auto &RenameDecl = **DeclsUnderCursor.begin();
- const auto *ID = RenameDecl.getIdentifier();
- if (!ID)
+ DeclarationName Name = RenameDecl.getDeclName();
+ if (!Name)
return makeError(ReasonToReject::UnsupportedSymbol);
- if (ID->getName() == RInputs.NewName)
+ SymbolName OldSymbolName(Name);
+ SymbolName NewSymbolName;
+ if (RInputs.NewName) {
+ NewSymbolName = SymbolName(*RInputs.NewName, AST.getLangOpts());
+ } else {
+ // If no new name is given, we are perfoming a pseudo rename for the
+ // prepareRename request to check if the rename is possible. Construct a
+ // new symbol name that has as many name pieces as the old name and is thus
+ // a valid new name.
+ std::vector<std::string> NewNamePieces = OldSymbolName.getNamePieces();
+ NewNamePieces[0] += "__clangd_rename_placeholder";
+ NewSymbolName = SymbolName(NewNamePieces);
+ }
+ if (OldSymbolName == NewSymbolName)
return makeError(ReasonToReject::SameName);
- auto Invalid = checkName(RenameDecl, RInputs.NewName);
+ auto Invalid = checkName(RenameDecl, NewSymbolName);
if (Invalid)
return makeError(std::move(*Invalid));
@@ -802,7 +847,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
// To make cross-file rename work for local symbol, we use a hybrid solution:
// - run AST-based rename on the main file;
// - run index-based rename on other affected files;
- auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName);
+ auto MainFileRenameEdit =
+ renameWithinFile(AST, RenameDecl, OldSymbolName, NewSymbolName);
if (!MainFileRenameEdit)
return MainFileRenameEdit.takeError();
@@ -827,6 +873,7 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
}
RenameResult Result;
Result.Target = CurrentIdentifier;
+ Result.OldName = RenameDecl.getNameAsString();
Edit MainFileEdits = Edit(MainFileCode, std::move(*MainFileRenameEdit));
for (const TextEdit &TE : MainFileEdits.asTextEdits())
Result.LocalChanges.push_back(TE.range);
@@ -847,7 +894,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
}
auto OtherFilesEdits = renameOutsideFile(
- RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index,
+ RenameDecl, RInputs.MainFilePath, OldSymbolName, NewSymbolName,
+ *RInputs.Index,
Opts.LimitFiles == 0 ? std::numeric_limits<size_t>::max()
: Opts.LimitFiles,
*RInputs.FS);
@@ -860,10 +908,11 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
return Result;
}
-llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
- llvm::StringRef InitialCode,
- std::vector<Range> Occurrences,
- llvm::StringRef NewName) {
+llvm::Expected<Edit>
+buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode,
+ std::vector<Range> Occurrences, SymbolName OldName,
+ SymbolName NewName,
+ const syntax::UnexpandedTokenBuffer &Tokens) {
trace::Span Tracer("BuildRenameEdit");
SPAN_ATTACH(Tracer, "file_path", AbsFilePath);
SPAN_ATTACH(Tracer, "rename_occurrences",
@@ -904,12 +953,36 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
}
+ const SourceManager &SM = Tokens.sourceManager();
+
tooling::Replacements RenameEdit;
for (const auto &R : OccurrencesOffsets) {
- auto ByteLength = R.second - R.first;
- if (auto Err = RenameEdit.add(
- tooling::Replacement(AbsFilePath, R.first, ByteLength, NewName)))
- return std::move(Err);
+ if (std::optional<std::string> Identifier = NewName.getSinglePiece()) {
+ auto ByteLength = R.second - R.first;
+ if (auto Err = RenameEdit.add(tooling::Replacement(
+ AbsFilePath, R.first, ByteLength, *Identifier)))
+ return std::move(Err);
+ } else {
+ SmallVector<SourceLocation> PieceLocations;
+ llvm::Error Error = findObjCSymbolSelectorPieces(
+ Tokens.tokens(), SM,
+ SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(R.first),
+ OldName, tooling::ObjCSymbolSelectorKind::Unknown, PieceLocations);
+ if (Error) {
+ // Ignore the error. We simply skip over all selectors that didn't
+ // match.
+ consumeError(std::move(Error));
+ continue;
+ }
+ assert(PieceLocations.size() == NewName.getNamePieces().size());
+ for (auto [Location, NewPiece] :
+ llvm::zip_equal(PieceLocations, NewName.getNamePieces())) {
+ tooling::Replacement NewReplacement(
+ SM, CharSourceRange::getTokenRange(Location), NewPiece);
+ if (auto Err = RenameEdit.add(NewReplacement))
+ return std::move(Err);
+ }
+ }
}
return Edit(InitialCode, std::move(RenameEdit));
}
@@ -928,13 +1001,13 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
// were inserted). If such a "near miss" is found, the rename is still
// possible
std::optional<std::vector<Range>>
-adjustRenameRanges(llvm::St...
[truncated]
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Alex Hoppen (ahoppen) ChangesThis adds support to rename Objective-C method declarations and calls to clangd. Patch is 54.98 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78872.diff 24 Files Affected:
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9b..5dfd12045b65738 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -844,14 +844,15 @@ void ClangdLSPServer::onWorkspaceSymbol(
}
void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
- Callback<std::optional<Range>> Reply) {
+ Callback<PrepareRenameResult> Reply) {
Server->prepareRename(
Params.textDocument.uri.file(), Params.position, /*NewName*/ std::nullopt,
Opts.Rename,
[Reply = std::move(Reply)](llvm::Expected<RenameResult> Result) mutable {
if (!Result)
return Reply(Result.takeError());
- return Reply(std::move(Result->Target));
+ PrepareRenameResult PrepareResult{Result->Target, Result->OldName};
+ return Reply(std::move(PrepareResult));
});
}
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 79579c22b788a9c..6a9f097d551ae6d 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -134,7 +134,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
void onWorkspaceSymbol(const WorkspaceSymbolParams &,
Callback<std::vector<SymbolInformation>>);
void onPrepareRename(const TextDocumentPositionParams &,
- Callback<std::optional<Range>>);
+ Callback<PrepareRenameResult>);
void onRename(const RenameParams &, Callback<WorkspaceEdit>);
void onHover(const TextDocumentPositionParams &,
Callback<std::optional<Hover>>);
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 6fb2641e8793db1..b04ebc7049c6619 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -578,8 +578,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
// prepareRename is latency-sensitive: we don't query the index, as we
// only need main-file references
auto Results =
- clangd::rename({Pos, NewName.value_or("__clangd_rename_placeholder"),
- InpAST->AST, File, /*FS=*/nullptr,
+ clangd::rename({Pos, NewName, InpAST->AST, File, /*FS=*/nullptr,
/*Index=*/nullptr, RenameOpts});
if (!Results) {
// LSP says to return null on failure, but that will result in a generic
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index a6370649f5ad1cf..0291e5d71d65c88 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -905,6 +905,10 @@ llvm::json::Value toJSON(const DocumentSymbol &S) {
return std::move(Result);
}
+llvm::json::Value toJSON(const PrepareRenameResult &R) {
+ return llvm::json::Object{{"range", R.range}, {"placeholder", R.placeholder}};
+}
+
llvm::json::Value toJSON(const WorkspaceEdit &WE) {
llvm::json::Object Result;
if (WE.changes) {
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index e88c804692568f5..274c7fe4391062b 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1004,6 +1004,17 @@ struct CodeActionParams {
};
bool fromJSON(const llvm::json::Value &, CodeActionParams &, llvm::json::Path);
+struct PrepareRenameResult {
+ /// The range of the string to rename.
+ Range range;
+ /// A placeholder text of the string content to be renamed.
+ ///
+ /// This is usueful to populate the rename field with an Objective-C selector
+ /// name (eg. `performAction:with:`) when renaming Objective-C methods.
+ std::string placeholder;
+};
+llvm::json::Value toJSON(const PrepareRenameResult &WE);
+
/// The edit should either provide changes or documentChanges. If the client
/// can handle versioned document edits and if documentChanges are present,
/// the latter are preferred over changes.
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 835038423fdf372..0081a69357b02f4 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -625,16 +625,16 @@ llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
return Identifiers;
}
-std::vector<Range> collectIdentifierRanges(llvm::StringRef Identifier,
- llvm::StringRef Content,
- const LangOptions &LangOpts) {
+std::vector<Range>
+collectIdentifierRanges(llvm::StringRef Identifier,
+ const syntax::UnexpandedTokenBuffer &Tokens) {
std::vector<Range> Ranges;
- lex(Content, LangOpts,
- [&](const syntax::Token &Tok, const SourceManager &SM) {
- if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
- return;
- Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
- });
+ const SourceManager &SM = Tokens.sourceManager();
+ for (const syntax::Token &Tok : Tokens.tokens()) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+ continue;
+ Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+ }
return Ranges;
}
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index a1bb44c17612021..f60683fc4f21c35 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -217,9 +217,9 @@ llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
const format::FormatStyle &Style);
/// Collects all ranges of the given identifier in the source code.
-std::vector<Range> collectIdentifierRanges(llvm::StringRef Identifier,
- llvm::StringRef Content,
- const LangOptions &LangOpts);
+std::vector<Range>
+collectIdentifierRanges(llvm::StringRef Identifier,
+ const syntax::UnexpandedTokenBuffer &Tokens);
/// Collects words from the source code.
/// Unlike collectIdentifiers:
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index bf838e53f2a21e7..24bd3c426b3e86b 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -173,9 +173,20 @@ std::optional<RelationKind> indexableRelation(const index::SymbolRelation &R) {
bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
- if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ bool PrefixComparison;
+ switch (NameKind) {
+ case DeclarationName::Identifier:
+ case DeclarationName::CXXConstructorName:
+ case DeclarationName::ObjCZeroArgSelector:
+ PrefixComparison = false;
+ break;
+ case DeclarationName::ObjCOneArgSelector:
+ case DeclarationName::ObjCMultiArgSelector:
+ PrefixComparison = true;
+ break;
+ default:
return false;
+ }
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
const auto &LO = AST.getLangOpts();
@@ -183,7 +194,17 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
- return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
+ std::string LexerSpelling = clang::Lexer::getSpelling(Tok, SM, LO);
+ if (PrefixComparison) {
+ // The lexer spelling at the source location is only the first label of an
+ // Objective-C selector, eg. if `StrName` is `performAction:with:`, then the
+ // token at the requested location is `performAction`. Re-building the
+ // entire selector from the lexer is too complicated here, so just perform
+ // a prefix comparison.
+ return StringRef(StrName).starts_with(LexerSpelling);
+ } else {
+ return StrName == LexerSpelling;
+ }
}
} // namespace
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 11f9e4627af7609..f639cbc9b4b1273 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -26,6 +26,8 @@
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolName.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
@@ -40,6 +42,8 @@ namespace clang {
namespace clangd {
namespace {
+using tooling::SymbolName;
+
std::optional<std::string> filePath(const SymbolLocation &Loc,
llvm::StringRef HintFilePath) {
if (!Loc)
@@ -517,22 +521,27 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
// Check if we can rename the given RenameDecl into NewName.
// Return details if the rename would produce a conflict.
std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+ const SymbolName &NewName) {
trace::Span Tracer("CheckName");
static constexpr trace::Metric InvalidNameMetric(
"rename_name_invalid", trace::Metric::Counter, "invalid_kind");
auto &ASTCtx = RenameDecl.getASTContext();
+ auto Identifier = NewName.getSinglePiece();
+ if (!Identifier) {
+ return std::nullopt;
+ }
std::optional<InvalidName> Result;
- if (isKeyword(NewName, ASTCtx.getLangOpts()))
- Result = InvalidName{InvalidName::Keywords, NewName.str()};
- else if (!mayBeValidIdentifier(NewName))
- Result = InvalidName{InvalidName::BadIdentifier, NewName.str()};
- else {
+ if (isKeyword(*Identifier, ASTCtx.getLangOpts())) {
+ Result = InvalidName{InvalidName::Keywords, *Identifier};
+ } else if (!mayBeValidIdentifier(*Identifier)) {
+ Result = InvalidName{InvalidName::BadIdentifier, *Identifier};
+ } else {
// Name conflict detection.
// Function conflicts are subtle (overloading), so ignore them.
if (RenameDecl.getKind() != Decl::Function &&
RenameDecl.getKind() != Decl::CXXMethod) {
- if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
+ if (auto *Conflict =
+ lookupSiblingWithName(ASTCtx, RenameDecl, *Identifier))
Result = InvalidName{
InvalidName::Conflict,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
@@ -546,7 +555,7 @@ std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
// AST-based rename, it renames all occurrences in the main file.
llvm::Expected<tooling::Replacements>
renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
- llvm::StringRef NewName) {
+ const SymbolName &OldName, const SymbolName &NewName) {
trace::Span Tracer("RenameWithinFile");
const SourceManager &SM = AST.getSourceManager();
@@ -569,9 +578,30 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
- if (auto Err = FilteredChanges.add(tooling::Replacement(
- SM, CharSourceRange::getTokenRange(RenameLoc), NewName)))
- return std::move(Err);
+ if (std::optional<std::string> Identifier = NewName.getSinglePiece()) {
+ tooling::Replacement NewReplacement(
+ SM, CharSourceRange::getTokenRange(RenameLoc), *Identifier);
+ if (auto Err = FilteredChanges.add(NewReplacement)) {
+ return std::move(Err);
+ }
+ continue;
+ }
+ SmallVector<SourceLocation> PieceLocations;
+ llvm::Error Error = findObjCSymbolSelectorPieces(
+ AST.getTokens().expandedTokens(), SM, RenameLoc, OldName,
+ tooling::ObjCSymbolSelectorKind::Unknown, PieceLocations);
+ if (Error) {
+ // Ignore the error. We simply skip over all selectors that didn't match.
+ consumeError(std::move(Error));
+ continue;
+ }
+ for (auto [Location, NewPiece] :
+ llvm::zip_equal(PieceLocations, NewName.getNamePieces())) {
+ tooling::Replacement NewReplacement(
+ SM, CharSourceRange::getTokenRange(Location), NewPiece);
+ if (auto Err = FilteredChanges.add(NewReplacement))
+ return std::move(Err);
+ }
}
return FilteredChanges;
}
@@ -664,8 +694,9 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
// there is no dirty buffer.
llvm::Expected<FileEdits>
renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
- llvm::StringRef NewName, const SymbolIndex &Index,
- size_t MaxLimitFiles, llvm::vfs::FileSystem &FS) {
+ SymbolName OldName, SymbolName NewName,
+ const SymbolIndex &Index, size_t MaxLimitFiles,
+ llvm::vfs::FileSystem &FS) {
trace::Span Tracer("RenameOutsideFile");
auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath,
Index, MaxLimitFiles);
@@ -683,10 +714,11 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
}
auto AffectedFileCode = (*ExpBuffer)->getBuffer();
- auto RenameRanges =
- adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(),
- std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts());
+ syntax::UnexpandedTokenBuffer Tokens(
+ AffectedFileCode, RenameDecl.getASTContext().getLangOpts());
+ std::optional<std::vector<Range>> RenameRanges =
+ adjustRenameRanges(Tokens, OldName.getNamePieces().front(),
+ std::move(FileAndOccurrences.second));
if (!RenameRanges) {
// Our heuristics fails to adjust rename ranges to the current state of
// the file, it is most likely the index is stale, so we give up the
@@ -695,8 +727,8 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
"(the index may be stale)",
FilePath);
}
- auto RenameEdit =
- buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName);
+ auto RenameEdit = buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges,
+ OldName, NewName, Tokens);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
RenameEdit.takeError());
@@ -779,12 +811,25 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
const auto &RenameDecl = **DeclsUnderCursor.begin();
- const auto *ID = RenameDecl.getIdentifier();
- if (!ID)
+ DeclarationName Name = RenameDecl.getDeclName();
+ if (!Name)
return makeError(ReasonToReject::UnsupportedSymbol);
- if (ID->getName() == RInputs.NewName)
+ SymbolName OldSymbolName(Name);
+ SymbolName NewSymbolName;
+ if (RInputs.NewName) {
+ NewSymbolName = SymbolName(*RInputs.NewName, AST.getLangOpts());
+ } else {
+ // If no new name is given, we are perfoming a pseudo rename for the
+ // prepareRename request to check if the rename is possible. Construct a
+ // new symbol name that has as many name pieces as the old name and is thus
+ // a valid new name.
+ std::vector<std::string> NewNamePieces = OldSymbolName.getNamePieces();
+ NewNamePieces[0] += "__clangd_rename_placeholder";
+ NewSymbolName = SymbolName(NewNamePieces);
+ }
+ if (OldSymbolName == NewSymbolName)
return makeError(ReasonToReject::SameName);
- auto Invalid = checkName(RenameDecl, RInputs.NewName);
+ auto Invalid = checkName(RenameDecl, NewSymbolName);
if (Invalid)
return makeError(std::move(*Invalid));
@@ -802,7 +847,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
// To make cross-file rename work for local symbol, we use a hybrid solution:
// - run AST-based rename on the main file;
// - run index-based rename on other affected files;
- auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName);
+ auto MainFileRenameEdit =
+ renameWithinFile(AST, RenameDecl, OldSymbolName, NewSymbolName);
if (!MainFileRenameEdit)
return MainFileRenameEdit.takeError();
@@ -827,6 +873,7 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
}
RenameResult Result;
Result.Target = CurrentIdentifier;
+ Result.OldName = RenameDecl.getNameAsString();
Edit MainFileEdits = Edit(MainFileCode, std::move(*MainFileRenameEdit));
for (const TextEdit &TE : MainFileEdits.asTextEdits())
Result.LocalChanges.push_back(TE.range);
@@ -847,7 +894,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
}
auto OtherFilesEdits = renameOutsideFile(
- RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index,
+ RenameDecl, RInputs.MainFilePath, OldSymbolName, NewSymbolName,
+ *RInputs.Index,
Opts.LimitFiles == 0 ? std::numeric_limits<size_t>::max()
: Opts.LimitFiles,
*RInputs.FS);
@@ -860,10 +908,11 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
return Result;
}
-llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
- llvm::StringRef InitialCode,
- std::vector<Range> Occurrences,
- llvm::StringRef NewName) {
+llvm::Expected<Edit>
+buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode,
+ std::vector<Range> Occurrences, SymbolName OldName,
+ SymbolName NewName,
+ const syntax::UnexpandedTokenBuffer &Tokens) {
trace::Span Tracer("BuildRenameEdit");
SPAN_ATTACH(Tracer, "file_path", AbsFilePath);
SPAN_ATTACH(Tracer, "rename_occurrences",
@@ -904,12 +953,36 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
}
+ const SourceManager &SM = Tokens.sourceManager();
+
tooling::Replacements RenameEdit;
for (const auto &R : OccurrencesOffsets) {
- auto ByteLength = R.second - R.first;
- if (auto Err = RenameEdit.add(
- tooling::Replacement(AbsFilePath, R.first, ByteLength, NewName)))
- return std::move(Err);
+ if (std::optional<std::string> Identifier = NewName.getSinglePiece()) {
+ auto ByteLength = R.second - R.first;
+ if (auto Err = RenameEdit.add(tooling::Replacement(
+ AbsFilePath, R.first, ByteLength, *Identifier)))
+ return std::move(Err);
+ } else {
+ SmallVector<SourceLocation> PieceLocations;
+ llvm::Error Error = findObjCSymbolSelectorPieces(
+ Tokens.tokens(), SM,
+ SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(R.first),
+ OldName, tooling::ObjCSymbolSelectorKind::Unknown, PieceLocations);
+ if (Error) {
+ // Ignore the error. We simply skip over all selectors that didn't
+ // match.
+ consumeError(std::move(Error));
+ continue;
+ }
+ assert(PieceLocations.size() == NewName.getNamePieces().size());
+ for (auto [Location, NewPiece] :
+ llvm::zip_equal(PieceLocations, NewName.getNamePieces())) {
+ tooling::Replacement NewReplacement(
+ SM, CharSourceRange::getTokenRange(Location), NewPiece);
+ if (auto Err = RenameEdit.add(NewReplacement))
+ return std::move(Err);
+ }
+ }
}
return Edit(InitialCode, std::move(RenameEdit));
}
@@ -928,13 +1001,13 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
// were inserted). If such a "near miss" is found, the rename is still
// possible
std::optional<std::vector<Range>>
-adjustRenameRanges(llvm::St...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff b9a1e2ab8dead4863834f70cdae56104ec92d041 5e4853d277dd94a414d2dbd050f98f3cec6a27ec -- clang/lib/Tooling/Refactoring/SymbolName.cpp clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/clangd/ClangdLSPServer.h clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/Protocol.cpp clang-tools-extra/clangd/Protocol.h clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/SourceCode.h clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/refactor/Rename.h clang-tools-extra/clangd/unittests/RenameTests.cpp clang-tools-extra/clangd/unittests/SourceCodeTests.cpp clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h clang/include/clang/Tooling/Refactoring/Rename/SymbolName.h clang/include/clang/Tooling/Syntax/Tokens.h clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp clang/lib/Tooling/Syntax/Tokens.cpp clang/unittests/Tooling/RefactoringActionRulesTest.cppView the diff from clang-format here.diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index ff64e17645..9bf3b94120 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -887,11 +887,10 @@ TEST(RenameTest, ObjCWithinFileRename) {
/// fail.
std::optional<llvm::StringRef> Expected;
};
- TestCase Tests[] = {
- // Simple rename
- {
- // Input
- R"cpp(
+ TestCase Tests[] = {// Simple rename
+ {
+ // Input
+ R"cpp(
@interface Foo
- (int)performA^ction:(int)action w^ith:(int)value;
@end
@@ -901,10 +900,10 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- // New name
- "performNewAction:by:",
- // Expected
- R"cpp(
+ // New name
+ "performNewAction:by:",
+ // Expected
+ R"cpp(
@interface Foo
- (int)performNewAction:(int)action by:(int)value;
@end
@@ -914,11 +913,11 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- },
- // Rename selector with macro
- {
- // Input
- R"cpp(
+ },
+ // Rename selector with macro
+ {
+ // Input
+ R"cpp(
#define mySelector - (int)performAction:(int)action with:(int)value
@interface Foo
^mySelector;
@@ -929,15 +928,15 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- // New name
- "performNewAction:by:",
- // Expected error
- std::nullopt,
- },
- // Rename selector in macro definition
- {
- // Input
- R"cpp(
+ // New name
+ "performNewAction:by:",
+ // Expected error
+ std::nullopt,
+ },
+ // Rename selector in macro definition
+ {
+ // Input
+ R"cpp(
#define mySelector - (int)perform^Action:(int)action with:(int)value
@interface Foo
mySelector;
@@ -948,18 +947,20 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- // New name
- "performNewAction:by:",
- // Expected error
- std::nullopt,
- },
- // Don't rename `@selector`
- // `@selector` is not tied to a single selector. Eg. there might be multiple
- // classes in the codebase that implement that selector. It's thus more like
- // a string literal and we shouldn't rename it.
- {
- // Input
- R"cpp(
+ // New name
+ "performNewAction:by:",
+ // Expected error
+ std::nullopt,
+ },
+ // Don't rename `@selector`
+ // `@selector` is not tied to a single selector. Eg. there
+ // might be multiple
+ // classes in the codebase that implement that selector.
+ // It's thus more like
+ // a string literal and we shouldn't rename it.
+ {
+ // Input
+ R"cpp(
@interface Foo
- (void)performA^ction:(int)action with:(int)value;
@end
@@ -969,10 +970,10 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- // New name
- "performNewAction:by:",
- // Expected
- R"cpp(
+ // New name
+ "performNewAction:by:",
+ // Expected
+ R"cpp(
@interface Foo
- (void)performNewAction:(int)action by:(int)value;
@end
@@ -982,11 +983,11 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- },
- // Fail if rename initiated inside @selector
- {
- // Input
- R"cpp(
+ },
+ // Fail if rename initiated inside @selector
+ {
+ // Input
+ R"cpp(
@interface Foo
- (void)performAction:(int)action with:(int)value;
@end
@@ -996,12 +997,11 @@ TEST(RenameTest, ObjCWithinFileRename) {
}
@end
)cpp",
- // New name
- "performNewAction:by:",
- // Expected
- std::nullopt,
- }
- };
+ // New name
+ "performNewAction:by:",
+ // Expected
+ std::nullopt,
+ }};
for (TestCase T : Tests) {
SCOPED_TRACE(T.Input);
Annotations Code(T.Input);
@@ -1017,8 +1017,8 @@ TEST(RenameTest, ObjCWithinFileRename) {
ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
EXPECT_EQ(
- applyEdits(std::move(RenameResult->GlobalChanges)).front().second,
- *Expected);
+ applyEdits(std::move(RenameResult->GlobalChanges)).front().second,
+ *Expected);
} else {
ASSERT_FALSE(bool(RenameResult));
consumeError(RenameResult.takeError());
@@ -1954,101 +1954,99 @@ TEST(CrossFileRenameTests, ObjC) {
llvm::StringRef ExpectedFooH;
llvm::StringRef ExpectedFooM;
};
- Case Cases[] = {
- // --- Zero arg selector
- {
- // Input
- R"cpp(
+ Case Cases[] = {// --- Zero arg selector
+ {
+ // Input
+ R"cpp(
@interface Foo
- (int)performA^ction;
@end
)cpp",
- R"cpp(
+ R"cpp(
@implementation Foo
- (int)performAction {
[self performAction];
}
@end
)cpp",
- // New name
- "performNewAction",
- // Expected
- R"cpp(
+ // New name
+ "performNewAction",
+ // Expected
+ R"cpp(
@interface Foo
- (int)performNewAction;
@end
)cpp",
- R"cpp(
+ R"cpp(
@implementation Foo
- (int)performNewAction {
[self performNewAction];
}
@end
)cpp",
- },
- // --- Single arg selector
- {
- // Input
- R"cpp(
+ },
+ // --- Single arg selector
+ {
+ // Input
+ R"cpp(
@interface Foo
- (int)performA^ction:(int)action;
@end
)cpp",
- R"cpp(
+ R"cpp(
@implementation Foo
- (int)performAction:(int)action {
[self performAction:action];
}
@end
)cpp",
- // New name
- "performNewAction:",
- // Expected
- R"cpp(
+ // New name
+ "performNewAction:",
+ // Expected
+ R"cpp(
@interface Foo
- (int)performNewAction:(int)action;
@end
)cpp",
- R"cpp(
+ R"cpp(
@implementation Foo
- (int)performNewAction:(int)action {
[self performNewAction:action];
}
@end
)cpp",
- },
- // --- Multi arg selector
- {
- // Input
- R"cpp(
+ },
+ // --- Multi arg selector
+ {
+ // Input
+ R"cpp(
@interface Foo
- (int)performA^ction:(int)action with:(int)value;
@end
)cpp",
- R"cpp(
+ R"cpp(
@implementation Foo
- (int)performAction:(int)action with:(int)value {
[self performAction:action with:value];
}
@end
)cpp",
- // New name
- "performNewAction:by:",
- // Expected
- R"cpp(
+ // New name
+ "performNewAction:by:",
+ // Expected
+ R"cpp(
@interface Foo
- (int)performNewAction:(int)action by:(int)value;
@end
)cpp",
- R"cpp(
+ R"cpp(
@implementation Foo
- (int)performNewAction:(int)action by:(int)value {
[self performNewAction:action by:value];
}
@end
)cpp",
- }
- };
+ }};
trace::TestTracer Tracer;
for (const auto &T : Cases) {
|
ahoppen
force-pushed
the
ahoppen/upstream-rename-objc
branch
from
048d6f0 to
0428ffc
Compare
bnbarham
approved these changes
Jan 31, 2024
There was a problem hiding this comment.
Nice 🥳! @sam-mccall / @HighCommander4 any thoughts/concerns?
This allows us to return the old name of an Objective-C method, which pre-populates the new name field in an editor.
ahoppen
force-pushed
the
ahoppen/upstream-rename-objc
branch
from
0428ffc to
868d2e0
Compare
ahoppen
force-pushed
the
ahoppen/upstream-rename-objc
branch
from
868d2e0 to
5e4853d
Compare
|
Hi @ahoppen ! Thanks a lot for the patch, but @DavidGoldman has also been working on this in #76466 and I'd rather follow up on that as we've already put in quite a lot of effort to get it into some decent shape and the functionality propsed by both of these approaches seem to be ~same (even the approaches), unless I am missing something. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support to rename Objective-C method declarations and calls to clangd.