[clang-tools-extra] edfc859 - Add support for renaming objc methods, even those with multiple selector pieces (#76466)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 15 12:53:41 PST 2024
Author: David Goldman
Date: 2024-02-15T15:53:37-05:00
New Revision: edfc859af89e44207bf499b5d702aa26a7357da4
URL: https://github.com/llvm/llvm-project/commit/edfc859af89e44207bf499b5d702aa26a7357da4
DIFF: https://github.com/llvm/llvm-project/commit/edfc859af89e44207bf499b5d702aa26a7357da4.diff
LOG: Add support for renaming objc methods, even those with multiple selector pieces (#76466)
This adds support for renaming Objective-C methods, which are unique since their method names can be split across multiple tokens.
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.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/test/rename.test
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9..3e097cbeed5929 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -844,14 +844,17 @@ 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;
+ PrepareResult.range = Result->Target;
+ PrepareResult.placeholder = Result->Placeholder;
+ return Reply(std::move(PrepareResult));
});
}
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 79579c22b788a9..6a9f097d551ae6 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/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index a6370649f5ad1c..f93a941a5c27e4 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1187,6 +1187,15 @@ bool fromJSON(const llvm::json::Value &Params, RenameParams &R,
O.map("position", R.position) && O.map("newName", R.newName);
}
+llvm::json::Value toJSON(const PrepareRenameResult &PRR) {
+ if (PRR.placeholder.empty())
+ return toJSON(PRR.range);
+ return llvm::json::Object{
+ {"range", toJSON(PRR.range)},
+ {"placeholder", PRR.placeholder},
+ };
+}
+
llvm::json::Value toJSON(const DocumentHighlight &DH) {
return llvm::json::Object{
{"range", toJSON(DH.range)},
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index e88c804692568f..dc3fdb5b5f6c9b 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1436,6 +1436,14 @@ struct RenameParams {
};
bool fromJSON(const llvm::json::Value &, RenameParams &, llvm::json::Path);
+struct PrepareRenameResult {
+ /// Range of the string to rename.
+ Range range;
+ /// Placeholder text to use in the editor if non-empty.
+ std::string placeholder;
+};
+llvm::json::Value toJSON(const PrepareRenameResult &PRR);
+
enum class DocumentHighlightKind { Text = 1, Read = 2, Write = 3 };
/// A document highlight is a range inside a text document which deserves
@@ -1969,6 +1977,28 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, const ASTNode &);
} // namespace clang
namespace llvm {
+
+template <> struct DenseMapInfo<clang::clangd::Range> {
+ using Range = clang::clangd::Range;
+ static inline Range getEmptyKey() {
+ static clang::clangd::Position Tomb{-1, -1};
+ static Range R{Tomb, Tomb};
+ return R;
+ }
+ static inline Range getTombstoneKey() {
+ static clang::clangd::Position Tomb{-2, -2};
+ static Range R{Tomb, Tomb};
+ return R;
+ }
+ static unsigned getHashValue(const Range &Val) {
+ return llvm::hash_combine(Val.start.line, Val.start.character, Val.end.line,
+ Val.end.character);
+ }
+ static bool isEqual(const Range &LHS, const Range &RHS) {
+ return std::tie(LHS.start, LHS.end) == std::tie(RHS.start, RHS.end);
+ }
+};
+
template <> struct format_provider<clang::clangd::Position> {
static void format(const clang::clangd::Position &Pos, raw_ostream &OS,
StringRef Style) {
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index bf838e53f2a21e..85b8fc549b016e 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,10 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
auto Name = ND.getDeclName();
const auto NameKind = Name.getNameKind();
if (NameKind != DeclarationName::Identifier &&
- NameKind != DeclarationName::CXXConstructorName)
+ NameKind != DeclarationName::CXXConstructorName &&
+ NameKind != DeclarationName::ObjCZeroArgSelector &&
+ NameKind != DeclarationName::ObjCOneArgSelector &&
+ NameKind != DeclarationName::ObjCMultiArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -182,8 +185,10 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
clang::Token Tok;
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
- auto StrName = Name.getAsString();
- return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
+ auto TokSpelling = clang::Lexer::getSpelling(Tok, SM, LO);
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&ND))
+ return TokSpelling == MD->getSelector().getNameForSlot(0);
+ return TokSpelling == Name.getAsString();
}
} // namespace
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 11f9e4627af760..650862c99bcd12 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -222,6 +222,11 @@ std::optional<ReasonToReject> renameable(const NamedDecl &RenameDecl,
return ReasonToReject::UnsupportedSymbol;
}
}
+ // We allow renaming ObjC methods although they don't have a simple
+ // identifier.
+ const auto *ID = RenameDecl.getIdentifier();
+ if (!ID && !isa<ObjCMethodDecl>(&RenameDecl))
+ return ReasonToReject::UnsupportedSymbol;
// Filter out symbols that are unsupported in both rename modes.
if (llvm::isa<NamespaceDecl>(&RenameDecl))
return ReasonToReject::UnsupportedSymbol;
@@ -498,7 +503,7 @@ llvm::Error makeError(InvalidName Reason) {
return error("invalid name: {0}", Message(Reason));
}
-static bool mayBeValidIdentifier(llvm::StringRef Ident) {
+static bool mayBeValidIdentifier(llvm::StringRef Ident, bool AllowColon) {
assert(llvm::json::isUTF8(Ident));
if (Ident.empty())
return false;
@@ -508,24 +513,45 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
+ if (AllowColon && C == ':')
+ continue;
if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
return false;
}
return true;
}
+std::string getName(const NamedDecl &RenameDecl) {
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl))
+ return MD->getSelector().getAsString();
+ if (const auto *ID = RenameDecl.getIdentifier())
+ return ID->getName().str();
+ return "";
+}
+
// 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) {
+llvm::Error checkName(const NamedDecl &RenameDecl, llvm::StringRef NewName,
+ llvm::StringRef OldName) {
trace::Span Tracer("CheckName");
static constexpr trace::Metric InvalidNameMetric(
"rename_name_invalid", trace::Metric::Counter, "invalid_kind");
+
+ if (OldName == NewName)
+ return makeError(ReasonToReject::SameName);
+
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
+ const auto Sel = MD->getSelector();
+ if (Sel.getNumArgs() != NewName.count(':') &&
+ NewName != "__clangd_rename_placeholder")
+ return makeError(InvalidName{InvalidName::BadIdentifier, NewName.str()});
+ }
+
auto &ASTCtx = RenameDecl.getASTContext();
std::optional<InvalidName> Result;
if (isKeyword(NewName, ASTCtx.getLangOpts()))
Result = InvalidName{InvalidName::Keywords, NewName.str()};
- else if (!mayBeValidIdentifier(NewName))
+ else if (!mayBeValidIdentifier(NewName, isa<ObjCMethodDecl>(&RenameDecl)))
Result = InvalidName{InvalidName::BadIdentifier, NewName.str()};
else {
// Name conflict detection.
@@ -538,11 +564,224 @@ std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
}
}
- if (Result)
+ if (Result) {
InvalidNameMetric.record(1, toString(Result->K));
+ return makeError(*Result);
+ }
+ return llvm::Error::success();
+}
+
+bool isSelectorLike(const syntax::Token &Cur, const syntax::Token &Next) {
+ return Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token &Next,
+ const SourceManager &SM,
+ llvm::StringRef SelectorName) {
+ if (SelectorName.empty())
+ return Cur.kind() == tok::colon;
+ return isSelectorLike(Cur, Next) && Cur.text(SM) == SelectorName;
+}
+
+// Scan through Tokens to find ranges for each selector fragment in Sel assuming
+// its first segment is located at Tokens.front().
+// The search will terminate upon seeing Terminator or a ; at the top level.
+std::optional<SymbolRange>
+findAllSelectorPieces(llvm::ArrayRef<syntax::Token> Tokens,
+ const SourceManager &SM, Selector Sel,
+ tok::TokenKind Terminator) {
+ assert(!Tokens.empty());
+
+ unsigned NumArgs = Sel.getNumArgs();
+ llvm::SmallVector<tok::TokenKind, 8> Closes;
+ std::vector<Range> SelectorPieces;
+ for (unsigned Index = 0, Last = Tokens.size(); Index < Last - 1; ++Index) {
+ const auto &Tok = Tokens[Index];
+
+ if (Closes.empty()) {
+ auto PieceCount = SelectorPieces.size();
+ if (PieceCount < NumArgs &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Sel.getNameForSlot(PieceCount))) {
+ // If 'foo:' instead of ':' (empty selector), we need to skip the ':'
+ // token after the name. We don't currently properly support empty
+ // selectors since we may lex them improperly due to ternary statements
+ // as well as don't properly support storing their ranges for edits.
+ if (!Sel.getNameForSlot(PieceCount).empty())
+ ++Index;
+ SelectorPieces.push_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+ continue;
+ }
+ // If we've found all pieces but the current token looks like another
+ // selector piece, it means the method being renamed is a strict prefix of
+ // the selector we've found - should be skipped.
+ if (SelectorPieces.size() >= NumArgs &&
+ isSelectorLike(Tok, Tokens[Index + 1]))
+ return std::nullopt;
+ }
+
+ if (Closes.empty() && Tok.kind() == Terminator)
+ return SelectorPieces.size() == NumArgs
+ ? std::optional(SymbolRange(SelectorPieces))
+ : std::nullopt;
+
+ switch (Tok.kind()) {
+ case tok::l_square:
+ Closes.push_back(tok::r_square);
+ break;
+ case tok::l_paren:
+ Closes.push_back(tok::r_paren);
+ break;
+ case tok::l_brace:
+ Closes.push_back(tok::r_brace);
+ break;
+ case tok::r_square:
+ case tok::r_paren:
+ case tok::r_brace:
+ if (Closes.empty() || Closes.back() != Tok.kind())
+ return std::nullopt;
+ Closes.pop_back();
+ break;
+ case tok::semi:
+ // top level ; terminates all statements.
+ if (Closes.empty())
+ return SelectorPieces.size() == NumArgs
+ ? std::optional(SymbolRange(SelectorPieces))
+ : std::nullopt;
+ break;
+ default:
+ break;
+ }
+ }
+ return std::nullopt;
+}
+
+/// Collects all ranges of the given identifier/selector in the source code.
+///
+/// If a selector is given, this does a full lex of the given source code in
+/// order to identify all selector fragments (e.g. in method exprs/decls) since
+/// they are non-contiguous.
+std::vector<SymbolRange> collectRenameIdentifierRanges(
+ llvm::StringRef Identifier, llvm::StringRef Content,
+ const LangOptions &LangOpts, std::optional<Selector> Selector) {
+ std::vector<SymbolRange> Ranges;
+ if (!Selector) {
+ auto IdentifierRanges =
+ collectIdentifierRanges(Identifier, Content, LangOpts);
+ for (const auto &R : IdentifierRanges)
+ Ranges.emplace_back(R);
+ return Ranges;
+ }
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Content.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+
+ // We track parens and brackets to ensure that we don't accidentally try
+ // parsing a method declaration or definition which isn't at the top level or
+ // similar looking expressions (e.g. an @selector() expression).
+ llvm::SmallVector<tok::TokenKind, 8> Closes;
+ llvm::StringRef FirstSelPiece = Selector->getNameForSlot(0);
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+ for (unsigned Index = 0; Index < Last; ++Index) {
+ const auto &Tok = Tokens[Index];
+
+ // Search for the first selector piece to begin a match, but make sure we're
+ // not in () to avoid the @selector(foo:bar:) case.
+ if ((Closes.empty() || Closes.back() == tok::r_square) &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM, FirstSelPiece)) {
+ // We found a candidate for our match, this might be a method call,
+ // declaration, or unrelated identifier eg:
+ // - [obj ^sel0: X sel1: Y ... ]
+ //
+ // or
+ //
+ // @interface Foo
+ // - (int)^sel0:(int)x sel1:(int)y;
+ // @end
+ //
+ // or
+ //
+ // @implementation Foo
+ // - (int)^sel0:(int)x sel1:(int)y {}
+ // @end
+ //
+ // but not @selector(sel0:sel1:)
+ //
+ // Check if we can find all the relevant selector peices starting from
+ // this token
+ auto SelectorRanges =
+ findAllSelectorPieces(ArrayRef(Tokens).slice(Index), SM, *Selector,
+ Closes.empty() ? tok::l_brace : Closes.back());
+ if (SelectorRanges)
+ Ranges.emplace_back(std::move(*SelectorRanges));
+ }
+
+ switch (Tok.kind()) {
+ case tok::l_square:
+ Closes.push_back(tok::r_square);
+ break;
+ case tok::l_paren:
+ Closes.push_back(tok::r_paren);
+ break;
+ case tok::r_square:
+ case tok::r_paren:
+ if (Closes.empty()) // Invalid code, give up on the rename.
+ return std::vector<SymbolRange>();
+
+ if (Closes.back() == Tok.kind())
+ Closes.pop_back();
+ break;
+ default:
+ break;
+ }
+ }
+ return Ranges;
+}
+
+clangd::Range tokenRangeForLoc(ParsedAST &AST, SourceLocation TokLoc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ const auto *Token = AST.getTokens().spelledTokenAt(TokLoc);
+ assert(Token && "rename expects spelled tokens");
+ clangd::Range Result;
+ Result.start = sourceLocToPosition(SM, Token->location());
+ Result.end = sourceLocToPosition(SM, Token->endLocation());
return Result;
}
+// AST-based ObjC method rename, it renames all occurrences in the main file
+// even for selectors which may have multiple tokens.
+llvm::Expected<tooling::Replacements>
+renameObjCMethodWithinFile(ParsedAST &AST, const ObjCMethodDecl *MD,
+ llvm::StringRef NewName,
+ std::vector<SourceLocation> SelectorOccurences) {
+ const SourceManager &SM = AST.getSourceManager();
+ auto Code = SM.getBufferData(SM.getMainFileID());
+ auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+ llvm::SmallVector<llvm::StringRef, 8> NewNames;
+ NewName.split(NewNames, ":");
+
+ std::vector<Range> Ranges;
+ const auto &LangOpts = MD->getASTContext().getLangOpts();
+ for (const auto &Loc : SelectorOccurences)
+ Ranges.push_back(tokenRangeForLoc(AST, Loc, SM, LangOpts));
+ auto FilePath = AST.tuPath();
+ auto RenameRanges = collectRenameIdentifierRanges(
+ RenameIdentifier, Code, LangOpts, MD->getSelector());
+ auto RenameEdit = buildRenameEdit(FilePath, Code, RenameRanges, NewNames);
+ if (!RenameEdit)
+ return error("failed to rename in file {0}: {1}", FilePath,
+ RenameEdit.takeError());
+ return RenameEdit->Replacements;
+}
+
// AST-based rename, it renames all occurrences in the main file.
llvm::Expected<tooling::Replacements>
renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
@@ -551,6 +790,7 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
const SourceManager &SM = AST.getSourceManager();
tooling::Replacements FilteredChanges;
+ std::vector<SourceLocation> Locs;
for (SourceLocation Loc : findOccurrencesWithinFile(AST, RenameDecl)) {
SourceLocation RenameLoc = Loc;
// We don't rename in any macro bodies, but we allow rename the symbol
@@ -569,8 +809,13 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
+ Locs.push_back(RenameLoc);
+ }
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl))
+ return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs));
+ for (const auto &Loc : Locs) {
if (auto Err = FilteredChanges.add(tooling::Replacement(
- SM, CharSourceRange::getTokenRange(RenameLoc), NewName)))
+ SM, CharSourceRange::getTokenRange(Loc), NewName)))
return std::move(Err);
}
return FilteredChanges;
@@ -681,12 +926,22 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
ExpBuffer.getError().message());
continue;
}
+ std::string RenameIdentifier = RenameDecl.getNameAsString();
+ std::optional<Selector> Selector = std::nullopt;
+ llvm::SmallVector<llvm::StringRef, 8> NewNames;
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
+ if (MD->getSelector().getNumArgs() > 1) {
+ RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+ Selector = MD->getSelector();
+ }
+ }
+ NewName.split(NewNames, ":");
auto AffectedFileCode = (*ExpBuffer)->getBuffer();
auto RenameRanges =
- adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(),
+ adjustRenameRanges(AffectedFileCode, RenameIdentifier,
std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts());
+ RenameDecl.getASTContext().getLangOpts(), Selector);
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
@@ -696,7 +951,7 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
FilePath);
}
auto RenameEdit =
- buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName);
+ buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewNames);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
RenameEdit.takeError());
@@ -724,7 +979,7 @@ bool impliesSimpleEdit(const Position &LHS, const Position &RHS) {
// *line* or *column*, but not both (increases chance of a robust mapping)
void findNearMiss(
std::vector<size_t> &PartialMatch, ArrayRef<Range> IndexedRest,
- ArrayRef<Range> LexedRest, int LexedIndex, int &Fuel,
+ ArrayRef<SymbolRange> LexedRest, int LexedIndex, int &Fuel,
llvm::function_ref<void(const std::vector<size_t> &)> MatchedCB) {
if (--Fuel < 0)
return;
@@ -734,7 +989,8 @@ void findNearMiss(
MatchedCB(PartialMatch);
return;
}
- if (impliesSimpleEdit(IndexedRest.front().start, LexedRest.front().start)) {
+ if (impliesSimpleEdit(IndexedRest.front().start,
+ LexedRest.front().range().start)) {
PartialMatch.push_back(LexedIndex);
findNearMiss(PartialMatch, IndexedRest.drop_front(), LexedRest.drop_front(),
LexedIndex + 1, Fuel, MatchedCB);
@@ -746,6 +1002,23 @@ void findNearMiss(
} // namespace
+SymbolRange::SymbolRange(Range R) : Ranges({R}) {}
+
+SymbolRange::SymbolRange(std::vector<Range> Ranges)
+ : Ranges(std::move(Ranges)) {}
+
+Range SymbolRange::range() const { return Ranges.front(); }
+
+bool operator==(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return LHS.Ranges == RHS.Ranges;
+}
+bool operator!=(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return !(LHS == RHS);
+}
+bool operator<(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return LHS.range() < RHS.range();
+}
+
llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
assert(!RInputs.Index == !RInputs.FS &&
"Index and FS must either both be specified or both null.");
@@ -778,15 +1051,12 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
+
const auto &RenameDecl = **DeclsUnderCursor.begin();
- const auto *ID = RenameDecl.getIdentifier();
- if (!ID)
- return makeError(ReasonToReject::UnsupportedSymbol);
- if (ID->getName() == RInputs.NewName)
- return makeError(ReasonToReject::SameName);
- auto Invalid = checkName(RenameDecl, RInputs.NewName);
+ std::string Placeholder = getName(RenameDecl);
+ auto Invalid = checkName(RenameDecl, RInputs.NewName, Placeholder);
if (Invalid)
- return makeError(std::move(*Invalid));
+ return std::move(Invalid);
auto Reject =
renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index, Opts);
@@ -806,27 +1076,39 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
if (!MainFileRenameEdit)
return MainFileRenameEdit.takeError();
+ llvm::DenseSet<Range> RenamedRanges;
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
+ // TODO: Insert the ranges from the ObjCMethodDecl/ObjCMessageExpr selector
+ // pieces which are being renamed. This will require us to make changes to
+ // locateDeclAt to preserve this AST node.
+ } else {
+ RenamedRanges.insert(CurrentIdentifier);
+ }
+
// Check the rename-triggering location is actually being renamed.
// This is a robustness check to avoid surprising rename results -- if the
// the triggering location is not actually the name of the node we identified
// (e.g. for broken code), then rename is likely not what users expect, so we
// reject this kind of rename.
- auto StartOffset = positionToOffset(MainFileCode, CurrentIdentifier.start);
- auto EndOffset = positionToOffset(MainFileCode, CurrentIdentifier.end);
- if (!StartOffset)
- return StartOffset.takeError();
- if (!EndOffset)
- return EndOffset.takeError();
- if (llvm::none_of(
- *MainFileRenameEdit,
- [&StartOffset, &EndOffset](const clang::tooling::Replacement &R) {
- return R.getOffset() == *StartOffset &&
- R.getLength() == *EndOffset - *StartOffset;
- })) {
- return makeError(ReasonToReject::NoSymbolFound);
+ for (const auto &Range : RenamedRanges) {
+ auto StartOffset = positionToOffset(MainFileCode, Range.start);
+ auto EndOffset = positionToOffset(MainFileCode, Range.end);
+ if (!StartOffset)
+ return StartOffset.takeError();
+ if (!EndOffset)
+ return EndOffset.takeError();
+ if (llvm::none_of(
+ *MainFileRenameEdit,
+ [&StartOffset, &EndOffset](const clang::tooling::Replacement &R) {
+ return R.getOffset() == *StartOffset &&
+ R.getLength() == *EndOffset - *StartOffset;
+ })) {
+ return makeError(ReasonToReject::NoSymbolFound);
+ }
}
RenameResult Result;
Result.Target = CurrentIdentifier;
+ Result.Placeholder = Placeholder;
Edit MainFileEdits = Edit(MainFileCode, std::move(*MainFileRenameEdit));
for (const TextEdit &TE : MainFileEdits.asTextEdits())
Result.LocalChanges.push_back(TE.range);
@@ -862,8 +1144,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
llvm::StringRef InitialCode,
- std::vector<Range> Occurrences,
- llvm::StringRef NewName) {
+ std::vector<SymbolRange> Occurrences,
+ llvm::ArrayRef<llvm::StringRef> NewNames) {
trace::Span Tracer("BuildRenameEdit");
SPAN_ATTACH(Tracer, "file_path", AbsFilePath);
SPAN_ATTACH(Tracer, "rename_occurrences",
@@ -893,22 +1175,38 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
return LastOffset;
};
- std::vector<std::pair</*start*/ size_t, /*end*/ size_t>> OccurrencesOffsets;
- for (const auto &R : Occurrences) {
- auto StartOffset = Offset(R.start);
- if (!StartOffset)
- return StartOffset.takeError();
- auto EndOffset = Offset(R.end);
- if (!EndOffset)
- return EndOffset.takeError();
- OccurrencesOffsets.push_back({*StartOffset, *EndOffset});
+ struct OccurrenceOffset {
+ size_t Start;
+ size_t End;
+ llvm::StringRef NewName;
+
+ OccurrenceOffset(size_t Start, size_t End, llvm::StringRef NewName)
+ : Start(Start), End(End), NewName(NewName) {}
+ };
+
+ std::vector<OccurrenceOffset> OccurrencesOffsets;
+ for (const auto &SR : Occurrences) {
+ for (auto [Range, NewName] : llvm::zip(SR.Ranges, NewNames)) {
+ auto StartOffset = Offset(Range.start);
+ if (!StartOffset)
+ return StartOffset.takeError();
+ auto EndOffset = Offset(Range.end);
+ if (!EndOffset)
+ return EndOffset.takeError();
+ // Nothing to do if the token/name hasn't changed.
+ auto CurName =
+ InitialCode.substr(*StartOffset, *EndOffset - *StartOffset);
+ if (CurName == NewName)
+ continue;
+ OccurrencesOffsets.emplace_back(*StartOffset, *EndOffset, NewName);
+ }
}
tooling::Replacements RenameEdit;
for (const auto &R : OccurrencesOffsets) {
- auto ByteLength = R.second - R.first;
+ auto ByteLength = R.End - R.Start;
if (auto Err = RenameEdit.add(
- tooling::Replacement(AbsFilePath, R.first, ByteLength, NewName)))
+ tooling::Replacement(AbsFilePath, R.Start, ByteLength, R.NewName)))
return std::move(Err);
}
return Edit(InitialCode, std::move(RenameEdit));
@@ -927,20 +1225,21 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
// ranges onto candidates in a plausible way (e.g. guess that lines
// were inserted). If such a "near miss" is found, the rename is still
// possible
-std::optional<std::vector<Range>>
+std::optional<std::vector<SymbolRange>>
adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
- std::vector<Range> Indexed, const LangOptions &LangOpts) {
+ std::vector<Range> Indexed, const LangOptions &LangOpts,
+ std::optional<Selector> Selector) {
trace::Span Tracer("AdjustRenameRanges");
assert(!Indexed.empty());
assert(llvm::is_sorted(Indexed));
- std::vector<Range> Lexed =
- collectIdentifierRanges(Identifier, DraftCode, LangOpts);
+ std::vector<SymbolRange> Lexed =
+ collectRenameIdentifierRanges(Identifier, DraftCode, LangOpts, Selector);
llvm::sort(Lexed);
return getMappedRanges(Indexed, Lexed);
}
-std::optional<std::vector<Range>> getMappedRanges(ArrayRef<Range> Indexed,
- ArrayRef<Range> Lexed) {
+std::optional<std::vector<SymbolRange>>
+getMappedRanges(ArrayRef<Range> Indexed, ArrayRef<SymbolRange> Lexed) {
trace::Span Tracer("GetMappedRanges");
assert(!Indexed.empty());
assert(llvm::is_sorted(Indexed));
@@ -955,7 +1254,7 @@ std::optional<std::vector<Range>> getMappedRanges(ArrayRef<Range> Indexed,
}
// Fast check for the special subset case.
if (std::includes(Indexed.begin(), Indexed.end(), Lexed.begin(), Lexed.end()))
- return Indexed.vec();
+ return Lexed.vec();
std::vector<size_t> Best;
size_t BestCost = std::numeric_limits<size_t>::max();
@@ -985,7 +1284,7 @@ std::optional<std::vector<Range>> getMappedRanges(ArrayRef<Range> Indexed,
SPAN_ATTACH(Tracer, "error", "Didn't find a near miss");
return std::nullopt;
}
- std::vector<Range> Mapped;
+ std::vector<SymbolRange> Mapped;
for (auto I : Best)
Mapped.push_back(Lexed[I]);
SPAN_ATTACH(Tracer, "mapped_ranges", static_cast<int64_t>(Mapped.size()));
@@ -1007,7 +1306,8 @@ std::optional<std::vector<Range>> getMappedRanges(ArrayRef<Range> Indexed,
//
diff [0]: line + 1 <- insert a line before edit 0.
//
diff [1]: column + 1 <- remove a line between edits 0 and 1, and insert a
// character on edit 1.
-size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed, ArrayRef<Range> Lexed,
+size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed,
+ ArrayRef<SymbolRange> Lexed,
ArrayRef<size_t> MappedIndex) {
assert(Indexed.size() == MappedIndex.size());
assert(llvm::is_sorted(Indexed));
@@ -1017,9 +1317,10 @@ size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed, ArrayRef<Range> Lexed,
int LastDLine = 0, LastDColumn = 0;
int Cost = 0;
for (size_t I = 0; I < Indexed.size(); ++I) {
- int DLine = Indexed[I].start.line - Lexed[MappedIndex[I]].start.line;
- int DColumn =
- Indexed[I].start.character - Lexed[MappedIndex[I]].start.character;
+ int DLine =
+ Indexed[I].start.line - Lexed[MappedIndex[I]].range().start.line;
+ int DColumn = Indexed[I].start.character -
+ Lexed[MappedIndex[I]].range().start.character;
int Line = Indexed[I].start.line;
if (Line != LastLine)
LastDColumn = 0; // column offsets don't carry cross lines.
diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h
index 91728ba59e5d84..be5c346ae150f7 100644
--- a/clang-tools-extra/clangd/refactor/Rename.h
+++ b/clang-tools-extra/clangd/refactor/Rename.h
@@ -11,7 +11,9 @@
#include "Protocol.h"
#include "SourceCode.h"
+#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Error.h"
#include <optional>
@@ -53,6 +55,8 @@ struct RenameInputs {
struct RenameResult {
// The range of the symbol that the user can attempt to rename.
Range Target;
+ // Placeholder text for the rename operation if non-empty.
+ std::string Placeholder;
// Rename occurrences for the current main file.
std::vector<Range> LocalChanges;
// Complete edits for the rename, including LocalChanges.
@@ -60,6 +64,25 @@ struct RenameResult {
FileEdits GlobalChanges;
};
+/// Represents a symbol range where the symbol can potentially have multiple
+/// tokens.
+struct SymbolRange {
+ /// Ranges for the tokens that make up the symbol's name.
+ /// Usually a single range, but there can be multiple ranges if the tokens for
+ /// the symbol are split, e.g. ObjC selectors.
+ std::vector<Range> Ranges;
+
+ SymbolRange(Range R);
+ SymbolRange(std::vector<Range> Ranges);
+
+ /// Returns the first range.
+ Range range() const;
+
+ friend bool operator==(const SymbolRange &LHS, const SymbolRange &RHS);
+ friend bool operator!=(const SymbolRange &LHS, const SymbolRange &RHS);
+ friend bool operator<(const SymbolRange &LHS, const SymbolRange &RHS);
+};
+
/// 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).
@@ -71,8 +94,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs);
/// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges.
llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
llvm::StringRef InitialCode,
- std::vector<Range> Occurrences,
- llvm::StringRef NewName);
+ std::vector<SymbolRange> Occurrences,
+ llvm::ArrayRef<llvm::StringRef> NewNames);
/// Adjusts indexed occurrences to match the current state of the file.
///
@@ -84,9 +107,10 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
/// The API assumes that Indexed contains only named occurrences (each
/// occurrence has the same length).
/// REQUIRED: Indexed is sorted.
-std::optional<std::vector<Range>>
+std::optional<std::vector<SymbolRange>>
adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
- std::vector<Range> Indexed, const LangOptions &LangOpts);
+ std::vector<Range> Indexed, const LangOptions &LangOpts,
+ std::optional<Selector> Selector);
/// Calculates the lexed occurrences that the given indexed occurrences map to.
/// Returns std::nullopt if we don't find a mapping.
@@ -94,15 +118,16 @@ adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
/// Exposed for testing only.
///
/// REQUIRED: Indexed and Lexed are sorted.
-std::optional<std::vector<Range>> getMappedRanges(ArrayRef<Range> Indexed,
- ArrayRef<Range> Lexed);
+std::optional<std::vector<SymbolRange>>
+getMappedRanges(ArrayRef<Range> Indexed, ArrayRef<SymbolRange> Lexed);
/// Evaluates how good the mapped result is. 0 indicates a perfect match.
///
/// Exposed for testing only.
///
/// REQUIRED: Indexed and Lexed are sorted, Indexed and MappedIndex have the
/// same size.
-size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed, ArrayRef<Range> Lexed,
+size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed,
+ ArrayRef<SymbolRange> Lexed,
ArrayRef<size_t> MappedIndex);
} // namespace clangd
diff --git a/clang-tools-extra/clangd/test/rename.test b/clang-tools-extra/clangd/test/rename.test
index 527b4263443a70..0dc1a103002b26 100644
--- a/clang-tools-extra/clangd/test/rename.test
+++ b/clang-tools-extra/clangd/test/rename.test
@@ -10,6 +10,8 @@
# CHECK: "id": 1,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": {
+# CHECK-NEXT: "placeholder": "foo",
+# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
# CHECK-NEXT: "character": 7,
# CHECK-NEXT: "line": 0
@@ -18,6 +20,7 @@
# CHECK-NEXT: "character": 4,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
+# CHECK-NEXT: }
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":2,"method":"textDocument/prepareRename","params":{"textDocument":{"uri":"test:///foo.cpp"},"position":{"line":0,"character":2}}}
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 9cbf59684fbc10..d91ef85d672711 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -103,6 +103,13 @@ std::string expectedResult(Annotations Test, llvm::StringRef NewName) {
return Result;
}
+std::vector<SymbolRange> symbolRanges(llvm::ArrayRef<Range> Ranges) {
+ std::vector<SymbolRange> Result;
+ for (const auto &R : Ranges)
+ Result.emplace_back(R);
+ return Result;
+}
+
TEST(RenameTest, WithinFileRename) {
// For each "^" this test moves cursor to its location and applies renaming
// while checking that all identifiers in [[]] ranges are also renamed.
@@ -876,6 +883,157 @@ TEST(RenameTest, WithinFileRename) {
}
}
+TEST(RenameTest, ObjCWithinFileRename) {
+ struct TestCase {
+ /// Annotated source code that should be renamed. Every point (indicated by
+ /// `^`) will be used as a rename location.
+ llvm::StringRef Input;
+ /// The new name that should be given to the rename locaitons.
+ llvm::StringRef NewName;
+ /// The expected rename source code or `nullopt` if we expect rename to
+ /// fail.
+ std::optional<llvm::StringRef> Expected;
+ };
+ TestCase Tests[] = {// Simple rename
+ {
+ // Input
+ R"cpp(
+ @interface Foo
+ - (int)performA^ction:(int)action w^ith:(int)value;
+ @end
+ @implementation Foo
+ - (int)performAc^tion:(int)action w^ith:(int)value {
+ return [self performAction:action with:value];
+ }
+ @end
+ )cpp",
+ // New name
+ "performNewAction:by:",
+ // Expected
+ R"cpp(
+ @interface Foo
+ - (int)performNewAction:(int)action by:(int)value;
+ @end
+ @implementation Foo
+ - (int)performNewAction:(int)action by:(int)value {
+ return [self performNewAction:action by:value];
+ }
+ @end
+ )cpp",
+ },
+ // Rename selector with macro
+ {
+ // Input
+ R"cpp(
+ #define mySelector - (int)performAction:(int)action with:(int)value
+ @interface Foo
+ ^mySelector;
+ @end
+ @implementation Foo
+ mySelector {
+ return [self performAction:action with:value];
+ }
+ @end
+ )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;
+ @end
+ @implementation Foo
+ mySelector {
+ return [self performAction:action with:value];
+ }
+ @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(
+ @interface Foo
+ - (void)performA^ction:(int)action with:(int)value;
+ @end
+ @implementation Foo
+ - (void)performAction:(int)action with:(int)value {
+ SEL mySelector = @selector(performAction:with:);
+ }
+ @end
+ )cpp",
+ // New name
+ "performNewAction:by:",
+ // Expected
+ R"cpp(
+ @interface Foo
+ - (void)performNewAction:(int)action by:(int)value;
+ @end
+ @implementation Foo
+ - (void)performNewAction:(int)action by:(int)value {
+ SEL mySelector = @selector(performAction:with:);
+ }
+ @end
+ )cpp",
+ },
+ // Fail if rename initiated inside @selector
+ {
+ // Input
+ R"cpp(
+ @interface Foo
+ - (void)performAction:(int)action with:(int)value;
+ @end
+ @implementation Foo
+ - (void)performAction:(int)action with:(int)value {
+ SEL mySelector = @selector(perfo^rmAction:with:);
+ }
+ @end
+ )cpp",
+ // New name
+ "performNewAction:by:",
+ // Expected
+ std::nullopt,
+ }};
+ for (TestCase T : Tests) {
+ SCOPED_TRACE(T.Input);
+ Annotations Code(T.Input);
+ auto TU = TestTU::withCode(Code.code());
+ TU.ExtraArgs.push_back("-xobjective-c");
+ auto AST = TU.build();
+ auto Index = TU.index();
+ for (const auto &RenamePos : Code.points()) {
+ auto RenameResult =
+ rename({RenamePos, T.NewName, AST, testPath(TU.Filename),
+ getVFSFromAST(AST), Index.get()});
+ if (std::optional<StringRef> Expected = T.Expected) {
+ ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+ ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
+ EXPECT_EQ(
+ applyEdits(std::move(RenameResult->GlobalChanges)).front().second,
+ *Expected);
+ } else {
+ ASSERT_FALSE(bool(RenameResult));
+ consumeError(RenameResult.takeError());
+ }
+ }
+ }
+}
+
TEST(RenameTest, Renameable) {
struct Case {
const char *Code;
@@ -926,12 +1084,38 @@ TEST(RenameTest, Renameable) {
void f(X x) {x+^+;})cpp",
"no symbol", HeaderFile},
- {R"cpp(// disallow rename on non-normal identifiers.
+ {R"cpp(
@interface Foo {}
- -(int) fo^o:(int)x; // Token is an identifier, but declaration name isn't a simple identifier.
+ - (int)[[fo^o]]:(int)x;
@end
)cpp",
- "not a supported kind", HeaderFile},
+ nullptr, HeaderFile, "newName:"},
+ {R"cpp(//disallow as : count must match
+ @interface Foo {}
+ - (int)fo^o:(int)x;
+ @end
+ )cpp",
+ "invalid name: the chosen name \"MockName\" is not a valid identifier",
+ HeaderFile},
+ {R"cpp(
+ @interface Foo {}
+ - (int)[[o^ne]]:(int)one two:(int)two;
+ @end
+ )cpp",
+ nullptr, HeaderFile, "a:two:"},
+ {R"cpp(
+ @interface Foo {}
+ - (int)[[o^ne]]:(int)one [[two]]:(int)two;
+ @end
+ )cpp",
+ nullptr, HeaderFile, "a:b:"},
+ {R"cpp(
+ @interface Foo {}
+ - (int)o^ne:(int)one [[two]]:(int)two;
+ @end
+ )cpp",
+ nullptr, HeaderFile, "one:three:"},
+
{R"cpp(
void foo(int);
void foo(char);
@@ -1137,7 +1321,7 @@ TEST(RenameTest, Renameable) {
int [[V^ar]];
}
)cpp",
- nullptr, !HeaderFile},
+ nullptr, !HeaderFile},
};
for (const auto& Case : Cases) {
@@ -1778,8 +1962,8 @@ TEST(CrossFileRenameTests, BuildRenameEdits) {
Annotations Code("[[😂]]");
auto LSPRange = Code.range();
llvm::StringRef FilePath = "/test/TestTU.cpp";
- llvm::StringRef NewName = "abc";
- auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName);
+ llvm::SmallVector<llvm::StringRef, 2> NewNames = {"abc"};
+ auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewNames);
ASSERT_TRUE(bool(Edit)) << Edit.takeError();
ASSERT_EQ(1UL, Edit->Replacements.size());
EXPECT_EQ(FilePath, Edit->Replacements.begin()->getFilePath());
@@ -1787,7 +1971,7 @@ TEST(CrossFileRenameTests, BuildRenameEdits) {
// Test invalid range.
LSPRange.end = {10, 0}; // out of range
- Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName);
+ Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewNames);
EXPECT_FALSE(Edit);
EXPECT_THAT(llvm::toString(Edit.takeError()),
testing::HasSubstr("fail to convert"));
@@ -1798,10 +1982,11 @@ TEST(CrossFileRenameTests, BuildRenameEdits) {
[[range]]
[[range]]
)cpp");
- Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName);
+ Edit =
+ buildRenameEdit(FilePath, T.code(), symbolRanges(T.ranges()), NewNames);
ASSERT_TRUE(bool(Edit)) << Edit.takeError();
EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second,
- expectedResult(T, NewName));
+ expectedResult(T, NewNames[0]));
}
TEST(CrossFileRenameTests, adjustRenameRanges) {
@@ -1855,8 +2040,9 @@ TEST(CrossFileRenameTests, adjustRenameRanges) {
for (const auto &T : Tests) {
SCOPED_TRACE(T.DraftCode);
Annotations Draft(T.DraftCode);
- auto ActualRanges = adjustRenameRanges(
- Draft.code(), "x", Annotations(T.IndexedCode).ranges(), LangOpts);
+ auto ActualRanges = adjustRenameRanges(Draft.code(), "x",
+ Annotations(T.IndexedCode).ranges(),
+ LangOpts, std::nullopt);
if (!ActualRanges)
EXPECT_THAT(Draft.ranges(), testing::IsEmpty());
else
@@ -1970,11 +2156,11 @@ TEST(RangePatchingHeuristic, GetMappedRanges) {
for (const auto &T : Tests) {
SCOPED_TRACE(T.IndexedCode);
auto Lexed = Annotations(T.LexedCode);
- auto LexedRanges = Lexed.ranges();
- std::vector<Range> ExpectedMatches;
+ auto LexedRanges = symbolRanges(Lexed.ranges());
+ std::vector<SymbolRange> ExpectedMatches;
for (auto P : Lexed.points()) {
- auto Match = llvm::find_if(LexedRanges, [&P](const Range& R) {
- return R.start == P;
+ auto Match = llvm::find_if(LexedRanges, [&P](const SymbolRange &R) {
+ return R.range().start == P;
});
ASSERT_NE(Match, LexedRanges.end());
ExpectedMatches.push_back(*Match);
@@ -2093,8 +2279,8 @@ TEST(CrossFileRenameTests, adjustmentCost) {
std::vector<size_t> MappedIndex;
for (size_t I = 0; I < C.ranges("lex").size(); ++I)
MappedIndex.push_back(I);
- EXPECT_EQ(renameRangeAdjustmentCost(C.ranges("idx"), C.ranges("lex"),
- MappedIndex),
+ EXPECT_EQ(renameRangeAdjustmentCost(
+ C.ranges("idx"), symbolRanges(C.ranges("lex")), MappedIndex),
T.ExpectedCost);
}
}
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1e7a30e69fabe0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -105,6 +105,9 @@ MATCHER(refRange, "") {
MATCHER_P2(OverriddenBy, Subject, Object, "") {
return arg == Relation{Subject.ID, RelationKind::OverriddenBy, Object.ID};
}
+MATCHER(isSpelled, "") {
+ return static_cast<bool>(arg.Kind & RefKind::Spelled);
+}
::testing::Matcher<const std::vector<Ref> &>
haveRanges(const std::vector<Range> Ranges) {
return ::testing::UnorderedPointwise(refRange(), Ranges);
@@ -524,6 +527,47 @@ TEST_F(SymbolCollectorTest, templateArgs) {
forCodeCompletion(false)))));
}
+TEST_F(SymbolCollectorTest, ObjCRefs) {
+ Annotations Header(R"(
+ @interface Person
+ - (void)$talk[[talk]];
+ - (void)$say[[say]]:(id)something;
+ @end
+ @interface Person (Category)
+ - (void)categoryMethod;
+ - (void)multiArg:(id)a method:(id)b;
+ @end
+ )");
+ Annotations Main(R"(
+ @implementation Person
+ - (void)$talk[[talk]] {}
+ - (void)$say[[say]]:(id)something {}
+ @end
+
+ void fff(Person *p) {
+ [p $talk[[talk]]];
+ [p $say[[say]]:0];
+ [p categoryMethod];
+ [p multiArg:0 method:0];
+ }
+ )");
+ CollectorOpts.RefFilter = RefKind::All;
+ CollectorOpts.CollectMainFileRefs = true;
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header.code(), Main.code(),
+ {"-fblocks", "-xobjective-c++", "-Wno-objc-root-class"});
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::talk").ID,
+ haveRanges(Main.ranges("talk")))));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Person::say:").ID,
+ haveRanges(Main.ranges("say")))));
+ EXPECT_THAT(Refs,
+ Contains(Pair(findSymbol(Symbols, "Person::categoryMethod").ID,
+ ElementsAre(isSpelled()))));
+ EXPECT_THAT(Refs,
+ Contains(Pair(findSymbol(Symbols, "Person::multiArg:method:").ID,
+ ElementsAre(isSpelled()))));
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
More information about the cfe-commits
mailing list