https://github.com/ahoppen updated
https://github.com/llvm/llvm-project/pull/82061
>From 1285081e8fc7d7ecd162ed8ec7201437dbd708da Mon Sep 17 00:00:00 2001
From: Alex Hoppen <ahop...@apple.com>
Date: Mon, 29 Jul 2024 17:33:43 -0700
Subject: [PATCH] [clangd] Use `RenameSymbolName` to represent Objective-C
selectors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is a cleaner design than using identifier and an optional `Selector`. It
also allows rename of Objective-C method names if no declaration is at hand and
thus no `Selector` instance can be formed. For example, when finding the ranges
to rename based on an index that’s not clangd’s built-in index.
---
clang-tools-extra/clangd/refactor/Rename.cpp | 101 ++++++++++++------
clang-tools-extra/clangd/refactor/Rename.h | 48 ++++++++-
.../clangd/unittests/RenameTests.cpp | 6 +-
3 files changed, 118 insertions(+), 37 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp
b/clang-tools-extra/clangd/refactor/Rename.cpp
index b7894b8918eed..26059167208aa 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,9 +18,11 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTTypeTraits.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclarationName.h"
#include "clang/AST/ParentMapContext.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/CharInfo.h"
@@ -597,11 +599,12 @@ bool isMatchingSelectorName(const syntax::Token &Cur,
const syntax::Token &Next,
// 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,
+ const SourceManager &SM, const RenameSymbolName &Name,
tok::TokenKind Terminator) {
assert(!Tokens.empty());
- unsigned NumArgs = Sel.getNumArgs();
+ ArrayRef<std::string> NamePieces = Name.getNamePieces();
+ unsigned NumArgs = NamePieces.size();
llvm::SmallVector<tok::TokenKind, 8> Closes;
std::vector<Range> SelectorPieces;
for (unsigned Index = 0, Last = Tokens.size(); Index < Last - 1; ++Index) {
@@ -611,12 +614,12 @@ findAllSelectorPieces(llvm::ArrayRef<syntax::Token>
Tokens,
auto PieceCount = SelectorPieces.size();
if (PieceCount < NumArgs &&
isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
- Sel.getNameForSlot(PieceCount))) {
+ NamePieces[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())
+ if (!NamePieces[PieceCount].empty())
++Index;
SelectorPieces.push_back(
halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
@@ -668,16 +671,17 @@ findAllSelectorPieces(llvm::ArrayRef<syntax::Token>
Tokens,
/// 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) {
+/// If `Name` is an Objective-C symbol name, 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(const RenameSymbolName &Name,
+ llvm::StringRef Content,
+ const LangOptions &LangOpts) {
std::vector<SymbolRange> Ranges;
- if (!Selector) {
+ if (auto SinglePiece = Name.getSinglePiece()) {
auto IdentifierRanges =
- collectIdentifierRanges(Identifier, Content, LangOpts);
+ collectIdentifierRanges(*SinglePiece, Content, LangOpts);
for (const auto &R : IdentifierRanges)
Ranges.emplace_back(R);
return Ranges;
@@ -691,7 +695,7 @@ std::vector<SymbolRange> collectRenameIdentifierRanges(
// 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);
+ llvm::StringRef FirstSelPiece = Name.getNamePieces()[0];
auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
unsigned Last = Tokens.size() - 1;
@@ -723,7 +727,7 @@ std::vector<SymbolRange> collectRenameIdentifierRanges(
// Check if we can find all the relevant selector peices starting from
// this token
auto SelectorRanges =
- findAllSelectorPieces(ArrayRef(Tokens).slice(Index), SM, *Selector,
+ findAllSelectorPieces(ArrayRef(Tokens).slice(Index), SM, Name,
Closes.empty() ? tok::l_brace : Closes.back());
if (SelectorRanges)
Ranges.emplace_back(std::move(*SelectorRanges));
@@ -770,7 +774,6 @@ renameObjCMethodWithinFile(ParsedAST &AST, const
ObjCMethodDecl *MD,
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, ":");
@@ -780,7 +783,7 @@ renameObjCMethodWithinFile(ParsedAST &AST, const
ObjCMethodDecl *MD,
Ranges.push_back(tokenRangeForLoc(AST, Loc, SM, LangOpts));
auto FilePath = AST.tuPath();
auto RenameRanges = collectRenameIdentifierRanges(
- RenameIdentifier, Code, LangOpts, MD->getSelector());
+ RenameSymbolName(MD->getDeclName()), Code, LangOpts);
auto RenameEdit = buildRenameEdit(FilePath, Code, RenameRanges, NewNames);
if (!RenameEdit)
return error("failed to rename in file {0}: {1}", FilePath,
@@ -942,21 +945,14 @@ renameOutsideFile(const NamedDecl &RenameDecl,
llvm::StringRef MainFilePath,
ExpBuffer.getError().message());
continue;
}
- std::string RenameIdentifier = RenameDecl.getNameAsString();
- std::optional<Selector> Selector = std::nullopt;
+ RenameSymbolName RenameName(RenameDecl.getDeclName());
llvm::SmallVector<llvm::StringRef, 8> NewNames;
- if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
- RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
- if (MD->getSelector().getNumArgs() > 1)
- Selector = MD->getSelector();
- }
NewName.split(NewNames, ":");
auto AffectedFileCode = (*ExpBuffer)->getBuffer();
- auto RenameRanges =
- adjustRenameRanges(AffectedFileCode, RenameIdentifier,
- std::move(FileAndOccurrences.second),
- RenameDecl.getASTContext().getLangOpts(), Selector);
+ auto RenameRanges = adjustRenameRanges(
+ AffectedFileCode, RenameName, std::move(FileAndOccurrences.second),
+ RenameDecl.getASTContext().getLangOpts());
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
@@ -1017,6 +1013,50 @@ void findNearMiss(
} // namespace
+RenameSymbolName::RenameSymbolName() : NamePieces({}) {}
+
+namespace {
+std::vector<std::string> extractNamePieces(const DeclarationName &DeclName) {
+ if (DeclName.getNameKind() ==
+ DeclarationName::NameKind::ObjCMultiArgSelector ||
+ DeclName.getNameKind() == DeclarationName::NameKind::ObjCOneArgSelector)
{
+ const Selector &Sel = DeclName.getObjCSelector();
+ std::vector<std::string> Result;
+ for (unsigned Slot = 0; Slot < Sel.getNumArgs(); ++Slot) {
+ Result.push_back(Sel.getNameForSlot(Slot).str());
+ }
+ return Result;
+ }
+ return {DeclName.getAsString()};
+}
+} // namespace
+
+RenameSymbolName::RenameSymbolName(const DeclarationName &DeclName)
+ : RenameSymbolName(extractNamePieces(DeclName)) {}
+
+RenameSymbolName::RenameSymbolName(ArrayRef<std::string> NamePieces) {
+ for (const auto &Piece : NamePieces)
+ this->NamePieces.push_back(Piece);
+}
+
+std::optional<std::string> RenameSymbolName::getSinglePiece() const {
+ if (getNamePieces().size() == 1) {
+ return NamePieces.front();
+ }
+ return std::nullopt;
+}
+
+std::string RenameSymbolName::getAsString() const {
+ std::string Result;
+ llvm::raw_string_ostream OS(Result);
+ this->print(OS);
+ return Result;
+}
+
+void RenameSymbolName::print(raw_ostream &OS) const {
+ llvm::interleave(NamePieces, OS, ":");
+}
+
SymbolRange::SymbolRange(Range R) : Ranges({R}) {}
SymbolRange::SymbolRange(std::vector<Range> Ranges)
@@ -1244,14 +1284,13 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef
AbsFilePath,
// were inserted). If such a "near miss" is found, the rename is still
// possible
std::optional<std::vector<SymbolRange>>
-adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
- std::vector<Range> Indexed, const LangOptions &LangOpts,
- std::optional<Selector> Selector) {
+adjustRenameRanges(llvm::StringRef DraftCode, const RenameSymbolName &Name,
+ std::vector<Range> Indexed, const LangOptions &LangOpts) {
trace::Span Tracer("AdjustRenameRanges");
assert(!Indexed.empty());
assert(llvm::is_sorted(Indexed));
std::vector<SymbolRange> Lexed =
- collectRenameIdentifierRanges(Identifier, DraftCode, LangOpts, Selector);
+ collectRenameIdentifierRanges(Name, DraftCode, LangOpts);
llvm::sort(Lexed);
return getMappedRanges(Indexed, Lexed);
}
diff --git a/clang-tools-extra/clangd/refactor/Rename.h
b/clang-tools-extra/clangd/refactor/Rename.h
index be5c346ae150f..0f61d9647e7c8 100644
--- a/clang-tools-extra/clangd/refactor/Rename.h
+++ b/clang-tools-extra/clangd/refactor/Rename.h
@@ -35,6 +35,49 @@ struct RenameOptions {
bool RenameVirtual = true;
};
+/// A name of a symbol that should be renamed.
+///
+/// Symbol's name can be composed of multiple strings. For example, Objective-C
+/// methods can contain multiple argument labels:
+///
+/// \code
+/// - (void) myMethodNamePiece: (int)x anotherNamePieces:(int)y;
+/// // ^~ string 0 ~~~~~ ^~ string 1 ~~~~~
+/// \endcode
+class RenameSymbolName {
+ llvm::SmallVector<std::string, 1> NamePieces;
+
+public:
+ RenameSymbolName();
+
+ /// Create a new \c SymbolName with the specified pieces.
+ explicit RenameSymbolName(ArrayRef<std::string> NamePieces);
+
+ explicit RenameSymbolName(const DeclarationName &Name);
+
+ ArrayRef<std::string> getNamePieces() const { return NamePieces; }
+
+ /// If this symbol consists of a single piece return it, otherwise return
+ /// `None`.
+ ///
+ /// Only symbols in Objective-C can consist of multiple pieces, so this
+ /// function always returns a value for non-Objective-C symbols.
+ std::optional<std::string> getSinglePiece() const;
+
+ /// Returns a human-readable version of this symbol name.
+ ///
+ /// If the symbol consists of multiple pieces (aka. it is an Objective-C
+ /// selector/method name), the pieces are separated by `:`, otherwise just an
+ /// identifier name.
+ std::string getAsString() const;
+
+ void print(raw_ostream &OS) const;
+
+ bool operator==(const RenameSymbolName &Other) const {
+ return NamePieces == Other.NamePieces;
+ }
+};
+
struct RenameInputs {
Position Pos; // the position triggering the rename
llvm::StringRef NewName;
@@ -108,9 +151,8 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef
AbsFilePath,
/// occurrence has the same length).
/// REQUIRED: Indexed is sorted.
std::optional<std::vector<SymbolRange>>
-adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
- std::vector<Range> Indexed, const LangOptions &LangOpts,
- std::optional<Selector> Selector);
+adjustRenameRanges(llvm::StringRef DraftCode, const RenameSymbolName &Name,
+ std::vector<Range> Indexed, const LangOptions &LangOpts);
/// Calculates the lexed occurrences that the given indexed occurrences map to.
/// Returns std::nullopt if we don't find a mapping.
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 65558440e36e3..2cb0722f7f285 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -2223,9 +2223,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(), RenameSymbolName(ArrayRef<std::string>{"x"}),
+ Annotations(T.IndexedCode).ranges(), LangOpts);
if (!ActualRanges)
EXPECT_THAT(Draft.ranges(), testing::IsEmpty());
else
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
