[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 27 11:50:13 PST 2023
https://github.com/DavidGoldman updated https://github.com/llvm/llvm-project/pull/76466
>From 4caf5b3c779bf18236b4b0be5bc7147d10339f2b Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Tue, 26 Dec 2023 15:59:01 -0500
Subject: [PATCH 1/6] [clangd][SymbolCollector] Treat ObjC methods as spelled
We'll treat multi-arg methods as spelled once we have full rename
support for them.
---
.../clangd/index/SymbolCollector.cpp | 6 ++-
.../clangd/unittests/SymbolCollectorTests.cpp | 42 +++++++++++++++++++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 7ef4b15febad22..336bc3506bb360 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -174,7 +174,9 @@ 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)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
@@ -183,6 +185,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (clang::Lexer::getRawToken(Loc, Tok, SM, LO))
return false;
auto StrName = Name.getAsString();
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&ND))
+ StrName = MD->getSelector().getNameForSlot(0).str();
return clang::Lexer::getSpelling(Tok, SM, LO) == StrName;
}
} // namespace
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 9cdc57ec01f327..1d4e1c1d75ea23 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,45 @@ 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(Not(isSpelled())))));
+}
+
TEST_F(SymbolCollectorTest, ObjCSymbols) {
const std::string Header = R"(
@interface Person
>From 1b6a09464ff5c7b1988fcb479d0a4ff876f696e6 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Tue, 26 Dec 2023 16:12:03 -0500
Subject: [PATCH 2/6] Run clang-format
---
.../clangd/unittests/SymbolCollectorTests.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 1d4e1c1d75ea23..5c20b950e4eac0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -560,10 +560,12 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
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(Not(isSpelled())))));
+ EXPECT_THAT(Refs,
+ Contains(Pair(findSymbol(Symbols, "Person::categoryMethod").ID,
+ ElementsAre(isSpelled()))));
+ EXPECT_THAT(Refs,
+ Contains(Pair(findSymbol(Symbols, "Person::multiArg:method:").ID,
+ ElementsAre(Not(isSpelled())))));
}
TEST_F(SymbolCollectorTest, ObjCSymbols) {
>From a25c68f61bde24030b259c723c7afb6161c95603 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Tue, 26 Dec 2023 17:12:23 -0500
Subject: [PATCH 3/6] ObjC method rename support
We now support renaming ObjC methods even with multiple selector pieces.
This requires clangd to lex each file to discover the selector pieces
since the index only stores the location of the initial selector token.
---
clang-tools-extra/clangd/ClangdLSPServer.cpp | 7 +-
clang-tools-extra/clangd/ClangdLSPServer.h | 2 +-
clang-tools-extra/clangd/Protocol.cpp | 10 +
clang-tools-extra/clangd/Protocol.h | 8 +
clang-tools-extra/clangd/SourceCode.cpp | 8 +
clang-tools-extra/clangd/SourceCode.h | 4 +
clang-tools-extra/clangd/refactor/Rename.cpp | 310 ++++++++++++++++---
clang-tools-extra/clangd/refactor/Rename.h | 42 ++-
8 files changed, 343 insertions(+), 48 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9..0e628cfc71c2de 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<std::optional<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..3a60b6e5db86bc 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<std::optional<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..29b99da27101a8 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1187,6 +1187,16 @@ 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) {
+ 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..18ac530e9c1a0f 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 set.
+ std::optional<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
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 835038423fdf37..73e12fc7ebde51 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -1243,6 +1243,14 @@ SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
return Loc;
}
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM, const LangOptions &LangOpts) {
+ auto TokenLength = clang::Lexer::MeasureTokenLength(TokLoc, SM, LangOpts);
+ clangd::Range Result;
+ Result.start = sourceLocToPosition(SM, TokLoc);
+ Result.end = sourceLocToPosition(SM, TokLoc.getLocWithOffset(TokenLength));
+ return Result;
+}
+
clangd::Range rangeTillEOL(llvm::StringRef Code, unsigned HashOffset) {
clangd::Range Result;
Result.end = Result.start = offsetToPosition(Code, HashOffset);
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index a1bb44c1761202..d671e05aedd76e 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -338,6 +338,10 @@ inline bool isReservedName(llvm::StringRef Name) {
SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
const SourceManager &SM);
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts);
+
/// Returns the range starting at offset and spanning the whole line. Escaped
/// newlines are not handled.
clangd::Range rangeTillEOL(llvm::StringRef Code, unsigned HashOffset);
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 11f9e4627af760..0e2cc08eec19b9 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -498,7 +498,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,7 +508,8 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
!isAsciiIdentifierStart(Ident.front(), AllowDollar))
return false;
for (char C : Ident) {
- if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
+ if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar) &&
+ (!AllowColon || C != ':'))
return false;
}
return true;
@@ -525,7 +526,7 @@ std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
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.
@@ -551,6 +552,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 +571,43 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
// }
if (!isInsideMainFile(RenameLoc, SM))
continue;
+ Locs.push_back(RenameLoc);
+ }
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
+ auto Code = SM.getBufferData(SM.getMainFileID());
+ auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
+ llvm::SmallVector<llvm::StringRef, 8> NewNames;
+ NewName.split(NewNames, ":");
+ if (NewNames.empty())
+ NewNames.push_back(NewName);
+ std::vector<Range> Ranges;
+ const auto &LangOpts = RenameDecl.getASTContext().getLangOpts();
+ for (const auto &Loc : Locs)
+ Ranges.push_back(tokenRangeForLoc(Loc, SM, LangOpts));
+ auto FilePath = AST.tuPath();
+ auto RenameRanges =
+ adjustRenameRanges(Code, RenameIdentifier,
+ std::move(Ranges),
+ RenameDecl.getASTContext().getLangOpts(),
+ MD->getSelector());
+ 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
+ // entire rename.
+ return error("Lex results don't match the content of file {0} "
+ "(selector handling may be incorrect)",
+ FilePath);
+ }
+ auto RenameEdit =
+ buildRenameEdit(FilePath, Code, *RenameRanges, NewNames);
+ if (!RenameEdit)
+ return error("failed to rename in file {0}: {1}", FilePath,
+ RenameEdit.takeError());
+ return RenameEdit->Replacements;
+ }
+ 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 +718,26 @@ 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, ":");
+ }
+ }
+ if (NewNames.empty()) {
+ NewNames.push_back(NewName);
+ }
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 +747,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 +775,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 +785,7 @@ 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 +797,12 @@ void findNearMiss(
} // namespace
+std::vector<SymbolRange>
+collectRenameIdentifierRanges(
+ llvm::StringRef Identifier, llvm::StringRef Content,
+ const LangOptions &LangOpts,
+ std::optional<Selector> Selector);
+
llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
assert(!RInputs.Index == !RInputs.FS &&
"Index and FS must either both be specified or both null.");
@@ -778,12 +835,25 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
+ std::optional<std::string> Placeholder;
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);
+ if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
+ const auto Sel = MD->getSelector();
+ if (Sel.getAsString() == RInputs.NewName)
+ return makeError(ReasonToReject::SameName);
+ if (Sel.getNumArgs() != RInputs.NewName.count(':') &&
+ RInputs.NewName != "__clangd_rename_placeholder")
+ return makeError(
+ InvalidName{InvalidName::BadIdentifier, RInputs.NewName.str()});
+ if (Sel.getNumArgs() > 1)
+ Placeholder = Sel.getAsString();
+ } else {
+ 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);
if (Invalid)
return makeError(std::move(*Invalid));
@@ -827,6 +897,7 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
}
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 +933,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::SmallVectorImpl<llvm::StringRef> &NewNames) {
trace::Span Tracer("BuildRenameEdit");
SPAN_ATTACH(Tracer, "file_path", AbsFilePath);
SPAN_ATTACH(Tracer, "rename_occurrences",
@@ -893,22 +964,36 @@ 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 It = SR.Ranges.begin(); It != SR.Ranges.end(); ++It) {
+ const auto &R = *It;
+ auto StartOffset = Offset(R.start);
+ if (!StartOffset)
+ return StartOffset.takeError();
+ auto EndOffset = Offset(R.end);
+ if (!EndOffset)
+ return EndOffset.takeError();
+ auto Index = It - SR.Ranges.begin();
+ OccurrencesOffsets.emplace_back(*StartOffset, *EndOffset,
+ NewNames[Index]);
+ }
}
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 +1012,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 +1041,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 +1071,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 +1093,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 +1104,9 @@ 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 DLine = Indexed[I].start.line - Lexed[MappedIndex[I]].range().start.line;
int DColumn =
- Indexed[I].start.character - Lexed[MappedIndex[I]].start.character;
+ 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.
@@ -1029,5 +1116,152 @@ size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed, ArrayRef<Range> Lexed,
return Cost;
}
+static 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 Cur.kind() == tok::identifier && Next.kind() == tok::colon &&
+ Cur.text(SM) == SelectorName &&
+ // We require the selector name and : to be contiguous.
+ // e.g. support `foo:` but not `foo :`.
+ Cur.endLocation() == Next.location();
+}
+
+static 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();
+}
+
+static void
+lex(llvm::StringRef Code, const LangOptions &LangOpts,
+ llvm::function_ref<void(const syntax::Token &, const SourceManager &SM)>
+ Action) {
+ // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
+ std::string NullTerminatedCode = Code.str();
+ SourceManagerForFile FileSM("mock_file_name.cpp", NullTerminatedCode);
+ auto &SM = FileSM.get();
+ for (const auto &Tok : syntax::tokenize(SM.getMainFileID(), SM, LangOpts))
+ Action(Tok, SM);
+}
+
+std::vector<SymbolRange>
+collectRenameIdentifierRanges(llvm::StringRef Identifier, llvm::StringRef Content,
+ const LangOptions &LangOpts,
+ std::optional<Selector> Selector) {
+ std::vector<SymbolRange> Ranges;
+ if (!Selector) {
+ lex(Content, LangOpts,
+ [&](const syntax::Token &Tok, const SourceManager &SM) {
+ if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
+ return;
+ Ranges.emplace_back(
+ halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+ });
+ 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();
+
+ auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
+ unsigned Last = Tokens.size() - 1;
+
+ // One parser state for top level and each `[]` pair, can be nested.
+ // Technically we should have a state or recursion for each ()/{} as well,
+ // but since we're expecting well formed code it shouldn't matter in practice.
+ struct ParserState {
+ unsigned ParenCount = 0;
+ unsigned BraceCount = 0;
+ std::vector<Range> Pieces;
+ };
+
+ // We have to track square brackets, parens and braces as we want to skip the
+ // tokens inside them. This ensures that we don't use identical selector
+ // pieces in inner message sends, blocks, lambdas and @selector expressions.
+ std::vector<ParserState> States = {ParserState()};
+ unsigned NumPieces = Selector->getNumArgs();
+
+ for (unsigned Index = 0; Index < Last; ++Index) {
+ auto &State = States.back();
+ auto &Pieces = State.Pieces;
+ const auto &Tok = Tokens[Index];
+ const auto Kind = Tok.kind();
+ auto PieceCount = Pieces.size();
+
+ if (State.ParenCount == 0) {
+ // Check for matches until we find all selector pieces.
+ if (PieceCount < NumPieces &&
+ isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
+ Selector->getNameForSlot(PieceCount))) {
+ if (!Selector->getNameForSlot(PieceCount).empty()) {
+ // Skip the ':' after the name. This ensures that it won't match a
+ // follow-up selector piece with an empty name.
+ ++Index;
+ }
+ Pieces.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+ continue;
+ }
+ // If we've found all pieces, we still need to try to consume more pieces
+ // as it's possible the selector being renamed is a prefix of this method
+ // name.
+ if (PieceCount >= NumPieces && isSelectorLike(Tok, Tokens[Index + 1])) {
+ ++Index;
+ Pieces.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
+ continue;
+ }
+ }
+ switch (Kind) {
+ case tok::r_square:
+ if (States.size() > 1) {
+ // All the selector pieces in the method expr have been found.
+ if (Pieces.size() == NumPieces) {
+ Ranges.emplace_back(std::move(Pieces));
+ }
+ States.pop_back();
+ }
+ break;
+ case tok::l_square:
+ States.push_back(ParserState());
+ break;
+ case tok::l_paren:
+ State.ParenCount++;
+ break;
+ case tok::r_paren:
+ if (State.ParenCount > 0) {
+ --State.ParenCount;
+ }
+ break;
+ case tok::r_brace:
+ if (State.BraceCount > 0) {
+ --State.BraceCount;
+ }
+ break;
+ case tok::l_brace:
+ ++State.BraceCount;
+ LLVM_FALLTHROUGH;
+ case tok::semi:
+ // At the top level scope we should only have method decls/defs.
+ if (States.size() == 1) {
+ // All the selector pieces in the method decl/def have been found.
+ if (Pieces.size() == NumPieces) {
+ Ranges.emplace_back(std::move(Pieces));
+ }
+ // ; and { end method decl/defs.
+ Pieces.clear();
+ }
+ break;
+ default:
+ break;
+ }
+ }
+ return Ranges;
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h
index 91728ba59e5d84..02bc1c8eadf663 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 set.
+ std::optional<std::string> Placeholder;
// Rename occurrences for the current main file.
std::vector<Range> LocalChanges;
// Complete edits for the rename, including LocalChanges.
@@ -60,6 +64,29 @@ struct RenameResult {
FileEdits GlobalChanges;
};
+/// Represents a symbol range where the symbol can potentially have multiple
+/// tokens.
+struct SymbolRange {
+ /// All ranges. If multiple, corresponds to an ObjC selector.
+ std::vector<Range> Ranges;
+
+ /// Returns the first range.
+ Range range() const { return Ranges.front(); }
+
+ SymbolRange(Range R) : Ranges({R}) {}
+ SymbolRange(std::vector<Range> Ranges) : Ranges(std::move(Ranges)) {}
+
+ friend bool operator==(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return LHS.Ranges == RHS.Ranges;
+ }
+ friend bool operator!=(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return !(LHS == RHS);
+ }
+ friend bool operator<(const SymbolRange &LHS, const SymbolRange &RHS) {
+ return LHS.range() < RHS.range();
+ }
+};
+
/// 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 +98,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::SmallVectorImpl<llvm::StringRef> &NewNames);
/// Adjusts indexed occurrences to match the current state of the file.
///
@@ -84,9 +111,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 +122,15 @@ 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
>From 76385b51db0e33a25df9646eb6d59f04a4f65af7 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Wed, 27 Dec 2023 14:30:06 -0500
Subject: [PATCH 4/6] Fix rename tests
---
.../clangd/index/SymbolCollector.cpp | 3 +-
clang-tools-extra/clangd/refactor/Rename.cpp | 11 +++++
clang-tools-extra/clangd/refactor/Rename.h | 2 +
.../clangd/unittests/RenameTests.cpp | 48 +++++++++++++------
.../clangd/unittests/SymbolCollectorTests.cpp | 2 +-
5 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 336bc3506bb360..7749727fa2ca1b 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -176,7 +176,8 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) {
if (NameKind != DeclarationName::Identifier &&
NameKind != DeclarationName::CXXConstructorName &&
NameKind != DeclarationName::ObjCZeroArgSelector &&
- NameKind != DeclarationName::ObjCOneArgSelector)
+ NameKind != DeclarationName::ObjCOneArgSelector &&
+ NameKind != DeclarationName::ObjCMultiArgSelector)
return false;
const auto &AST = ND.getASTContext();
const auto &SM = AST.getSourceManager();
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 0e2cc08eec19b9..4f945f74e85000 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -797,6 +797,13 @@ void findNearMiss(
} // namespace
+std::vector<SymbolRange> symbolRanges(const std::vector<Range> Ranges) {
+ std::vector<SymbolRange> Result;
+ for (const auto &R : Ranges)
+ Result.emplace_back(R);
+ return Result;
+}
+
std::vector<SymbolRange>
collectRenameIdentifierRanges(
llvm::StringRef Identifier, llvm::StringRef Content,
@@ -984,6 +991,10 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
if (!EndOffset)
return EndOffset.takeError();
auto Index = It - SR.Ranges.begin();
+ // Nothing to do if the token/name hasn't changed.
+ auto CurName = InitialCode.substr(*StartOffset, *EndOffset - *StartOffset);
+ if (CurName == NewNames[Index])
+ continue;
OccurrencesOffsets.emplace_back(*StartOffset, *EndOffset,
NewNames[Index]);
}
diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h
index 02bc1c8eadf663..c96363324efb6f 100644
--- a/clang-tools-extra/clangd/refactor/Rename.h
+++ b/clang-tools-extra/clangd/refactor/Rename.h
@@ -87,6 +87,8 @@ struct SymbolRange {
}
};
+std::vector<SymbolRange> symbolRanges(const std::vector<Range> Ranges);
+
/// 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).
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 9cbf59684fbc10..a4593142567377 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -926,12 +926,31 @@ 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(
void foo(int);
void foo(char);
@@ -1778,8 +1797,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 +1806,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 +1817,10 @@ 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) {
@@ -1856,7 +1875,7 @@ TEST(CrossFileRenameTests, adjustRenameRanges) {
SCOPED_TRACE(T.DraftCode);
Annotations Draft(T.DraftCode);
auto ActualRanges = adjustRenameRanges(
- Draft.code(), "x", Annotations(T.IndexedCode).ranges(), LangOpts);
+ Draft.code(), "x", Annotations(T.IndexedCode).ranges(), LangOpts, std::nullopt);
if (!ActualRanges)
EXPECT_THAT(Draft.ranges(), testing::IsEmpty());
else
@@ -1970,11 +1989,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,7 +2112,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"),
+ 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 5c20b950e4eac0..1e7a30e69fabe0 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -565,7 +565,7 @@ TEST_F(SymbolCollectorTest, ObjCRefs) {
ElementsAre(isSpelled()))));
EXPECT_THAT(Refs,
Contains(Pair(findSymbol(Symbols, "Person::multiArg:method:").ID,
- ElementsAre(Not(isSpelled())))));
+ ElementsAre(isSpelled()))));
}
TEST_F(SymbolCollectorTest, ObjCSymbols) {
>From 1fc12b7c29a64e709a2630414269cb1c3217f0db Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Wed, 27 Dec 2023 14:35:53 -0500
Subject: [PATCH 5/6] Run clang format
---
clang-tools-extra/clangd/refactor/Rename.cpp | 3 ++-
.../clangd/unittests/RenameTests.cpp | 20 ++++++++++---------
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 4f945f74e85000..26dd5822576c2d 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -992,7 +992,8 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
return EndOffset.takeError();
auto Index = It - SR.Ranges.begin();
// Nothing to do if the token/name hasn't changed.
- auto CurName = InitialCode.substr(*StartOffset, *EndOffset - *StartOffset);
+ auto CurName =
+ InitialCode.substr(*StartOffset, *EndOffset - *StartOffset);
if (CurName == NewNames[Index])
continue;
OccurrencesOffsets.emplace_back(*StartOffset, *EndOffset,
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index a4593142567377..54118adaa48ad2 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -937,7 +937,8 @@ TEST(RenameTest, Renameable) {
- (int)fo^o:(int)x;
@end
)cpp",
- "invalid name: the chosen name \"MockName\" is not a valid identifier", HeaderFile},
+ "invalid name: the chosen name \"MockName\" is not a valid identifier",
+ HeaderFile},
{R"cpp(
@interface Foo {}
- (int)[[o^ne]]:(int)one two:(int)two;
@@ -1156,7 +1157,7 @@ TEST(RenameTest, Renameable) {
int [[V^ar]];
}
)cpp",
- nullptr, !HeaderFile},
+ nullptr, !HeaderFile},
};
for (const auto& Case : Cases) {
@@ -1817,7 +1818,8 @@ TEST(CrossFileRenameTests, BuildRenameEdits) {
[[range]]
[[range]]
)cpp");
- Edit = buildRenameEdit(FilePath, T.code(), symbolRanges(T.ranges()), NewNames);
+ 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, NewNames[0]));
@@ -1874,8 +1876,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, std::nullopt);
+ auto ActualRanges = adjustRenameRanges(Draft.code(), "x",
+ Annotations(T.IndexedCode).ranges(),
+ LangOpts, std::nullopt);
if (!ActualRanges)
EXPECT_THAT(Draft.ranges(), testing::IsEmpty());
else
@@ -1992,7 +1995,7 @@ TEST(RangePatchingHeuristic, GetMappedRanges) {
auto LexedRanges = symbolRanges(Lexed.ranges());
std::vector<SymbolRange> ExpectedMatches;
for (auto P : Lexed.points()) {
- auto Match = llvm::find_if(LexedRanges, [&P](const SymbolRange& R) {
+ auto Match = llvm::find_if(LexedRanges, [&P](const SymbolRange &R) {
return R.range().start == P;
});
ASSERT_NE(Match, LexedRanges.end());
@@ -2112,9 +2115,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"),
- symbolRanges(C.ranges("lex")),
- MappedIndex),
+ EXPECT_EQ(renameRangeAdjustmentCost(
+ C.ranges("idx"), symbolRanges(C.ranges("lex")), MappedIndex),
T.ExpectedCost);
}
}
>From 0ffdb0e27dd835c4b880a09d38040fed5a654ae2 Mon Sep 17 00:00:00 2001
From: David Goldman <davg at google.com>
Date: Wed, 27 Dec 2023 14:49:56 -0500
Subject: [PATCH 6/6] Properly run clang format
---
clang-tools-extra/clangd/ClangdLSPServer.cpp | 5 +-
clang-tools-extra/clangd/SourceCode.cpp | 3 +-
clang-tools-extra/clangd/SourceCode.h | 3 +-
clang-tools-extra/clangd/refactor/Rename.cpp | 57 +++++++++-----------
clang-tools-extra/clangd/refactor/Rename.h | 15 +++---
5 files changed, 40 insertions(+), 43 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 0e628cfc71c2de..641266b76c24b0 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -843,8 +843,9 @@ void ClangdLSPServer::onWorkspaceSymbol(
});
}
-void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
- Callback<std::optional<PrepareRenameResult>> Reply) {
+void ClangdLSPServer::onPrepareRename(
+ const TextDocumentPositionParams &Params,
+ Callback<std::optional<PrepareRenameResult>> Reply) {
Server->prepareRename(
Params.textDocument.uri.file(), Params.position, /*NewName*/ std::nullopt,
Opts.Rename,
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 73e12fc7ebde51..ac20ed095bcab8 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -1243,7 +1243,8 @@ SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
return Loc;
}
-clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM, const LangOptions &LangOpts) {
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM,
+ const LangOptions &LangOpts) {
auto TokenLength = clang::Lexer::MeasureTokenLength(TokLoc, SM, LangOpts);
clangd::Range Result;
Result.start = sourceLocToPosition(SM, TokLoc);
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index d671e05aedd76e..efe6d66c168771 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -338,8 +338,7 @@ inline bool isReservedName(llvm::StringRef Name) {
SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
const SourceManager &SM);
-clangd::Range tokenRangeForLoc(SourceLocation TokLoc,
- const SourceManager &SM,
+clangd::Range tokenRangeForLoc(SourceLocation TokLoc, const SourceManager &SM,
const LangOptions &LangOpts);
/// Returns the range starting at offset and spanning the whole line. Escaped
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 26dd5822576c2d..e5686c963af093 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -585,11 +585,9 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
for (const auto &Loc : Locs)
Ranges.push_back(tokenRangeForLoc(Loc, SM, LangOpts));
auto FilePath = AST.tuPath();
- auto RenameRanges =
- adjustRenameRanges(Code, RenameIdentifier,
- std::move(Ranges),
- RenameDecl.getASTContext().getLangOpts(),
- MD->getSelector());
+ auto RenameRanges = adjustRenameRanges(
+ Code, RenameIdentifier, std::move(Ranges),
+ RenameDecl.getASTContext().getLangOpts(), MD->getSelector());
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
@@ -598,8 +596,7 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
"(selector handling may be incorrect)",
FilePath);
}
- auto RenameEdit =
- buildRenameEdit(FilePath, Code, *RenameRanges, NewNames);
+ auto RenameEdit = buildRenameEdit(FilePath, Code, *RenameRanges, NewNames);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
RenameEdit.takeError());
@@ -736,8 +733,7 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
auto RenameRanges =
adjustRenameRanges(AffectedFileCode, RenameIdentifier,
std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts(),
- Selector);
+ 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
@@ -785,7 +781,8 @@ void findNearMiss(
MatchedCB(PartialMatch);
return;
}
- if (impliesSimpleEdit(IndexedRest.front().start, LexedRest.front().range().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);
@@ -804,11 +801,9 @@ std::vector<SymbolRange> symbolRanges(const std::vector<Range> Ranges) {
return Result;
}
-std::vector<SymbolRange>
-collectRenameIdentifierRanges(
- llvm::StringRef Identifier, llvm::StringRef Content,
- const LangOptions &LangOpts,
- std::optional<Selector> Selector);
+std::vector<SymbolRange> collectRenameIdentifierRanges(
+ llvm::StringRef Identifier, llvm::StringRef Content,
+ const LangOptions &LangOpts, std::optional<Selector> Selector);
llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
assert(!RInputs.Index == !RInputs.FS &&
@@ -938,10 +933,10 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) {
return Result;
}
-llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
- llvm::StringRef InitialCode,
- std::vector<SymbolRange> Occurrences,
- llvm::SmallVectorImpl<llvm::StringRef> &NewNames) {
+llvm::Expected<Edit>
+buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode,
+ std::vector<SymbolRange> Occurrences,
+ llvm::SmallVectorImpl<llvm::StringRef> &NewNames) {
trace::Span Tracer("BuildRenameEdit");
SPAN_ATTACH(Tracer, "file_path", AbsFilePath);
SPAN_ATTACH(Tracer, "rename_occurrences",
@@ -976,8 +971,8 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
size_t End;
llvm::StringRef NewName;
- OccurrenceOffset(size_t Start, size_t End, llvm::StringRef NewName) :
- Start(Start), End(End), NewName(NewName) {}
+ OccurrenceOffset(size_t Start, size_t End, llvm::StringRef NewName)
+ : Start(Start), End(End), NewName(NewName) {}
};
std::vector<OccurrenceOffset> OccurrencesOffsets;
@@ -1037,8 +1032,8 @@ adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
return getMappedRanges(Indexed, Lexed);
}
-std::optional<std::vector<SymbolRange>> getMappedRanges(ArrayRef<Range> Indexed,
- ArrayRef<SymbolRange> 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));
@@ -1116,9 +1111,10 @@ size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed,
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]].range().start.line;
- int DColumn =
- Indexed[I].start.character - Lexed[MappedIndex[I]].range().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.
@@ -1161,10 +1157,9 @@ lex(llvm::StringRef Code, const LangOptions &LangOpts,
Action(Tok, SM);
}
-std::vector<SymbolRange>
-collectRenameIdentifierRanges(llvm::StringRef Identifier, llvm::StringRef Content,
- const LangOptions &LangOpts,
- std::optional<Selector> Selector) {
+std::vector<SymbolRange> collectRenameIdentifierRanges(
+ llvm::StringRef Identifier, llvm::StringRef Content,
+ const LangOptions &LangOpts, std::optional<Selector> Selector) {
std::vector<SymbolRange> Ranges;
if (!Selector) {
lex(Content, LangOpts,
@@ -1256,7 +1251,7 @@ collectRenameIdentifierRanges(llvm::StringRef Identifier, llvm::StringRef Conten
break;
case tok::l_brace:
++State.BraceCount;
- LLVM_FALLTHROUGH;
+ LLVM_FALLTHROUGH;
case tok::semi:
// At the top level scope we should only have method decls/defs.
if (States.size() == 1) {
diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h
index c96363324efb6f..3951d7d559dcb0 100644
--- a/clang-tools-extra/clangd/refactor/Rename.h
+++ b/clang-tools-extra/clangd/refactor/Rename.h
@@ -98,10 +98,10 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs);
/// NewName.
/// Exposed for testing only.
/// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges.
-llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
- llvm::StringRef InitialCode,
- std::vector<SymbolRange> Occurrences,
- llvm::SmallVectorImpl<llvm::StringRef> &NewNames);
+llvm::Expected<Edit>
+buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode,
+ std::vector<SymbolRange> Occurrences,
+ llvm::SmallVectorImpl<llvm::StringRef> &NewNames);
/// Adjusts indexed occurrences to match the current state of the file.
///
@@ -124,15 +124,16 @@ adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
/// Exposed for testing only.
///
/// REQUIRED: Indexed and Lexed are sorted.
-std::optional<std::vector<SymbolRange>> getMappedRanges(ArrayRef<Range> Indexed,
- ArrayRef<SymbolRange> 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<SymbolRange> Lexed,
+size_t renameRangeAdjustmentCost(ArrayRef<Range> Indexed,
+ ArrayRef<SymbolRange> Lexed,
ArrayRef<size_t> MappedIndex);
} // namespace clangd
More information about the cfe-commits
mailing list