diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index a87da252b7a7e..5dfd12045b657 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -844,14 +844,15 @@ void ClangdLSPServer::onWorkspaceSymbol( } void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params, - Callback> Reply) { + Callback Reply) { Server->prepareRename( Params.textDocument.uri.file(), Params.position, /*NewName*/ std::nullopt, Opts.Rename, [Reply = std::move(Reply)](llvm::Expected Result) mutable { if (!Result) return Reply(Result.takeError()); - return Reply(std::move(Result->Target)); + PrepareRenameResult PrepareResult{Result->Target, Result->OldName}; + return Reply(std::move(PrepareResult)); }); } diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 79579c22b788a..6a9f097d551ae 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>); void onPrepareRename(const TextDocumentPositionParams &, - Callback>); + Callback); void onRename(const RenameParams &, Callback); void onHover(const TextDocumentPositionParams &, Callback>); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 6fb2641e8793d..b04ebc7049c66 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -578,8 +578,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos, // prepareRename is latency-sensitive: we don't query the index, as we // only need main-file references auto Results = - clangd::rename({Pos, NewName.value_or("__clangd_rename_placeholder"), - InpAST->AST, File, /*FS=*/nullptr, + clangd::rename({Pos, NewName, InpAST->AST, File, /*FS=*/nullptr, /*Index=*/nullptr, RenameOpts}); if (!Results) { // LSP says to return null on failure, but that will result in a generic diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index a6370649f5ad1..0291e5d71d65c 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -905,6 +905,10 @@ llvm::json::Value toJSON(const DocumentSymbol &S) { return std::move(Result); } +llvm::json::Value toJSON(const PrepareRenameResult &R) { + return llvm::json::Object{{"range", R.range}, {"placeholder", R.placeholder}}; +} + llvm::json::Value toJSON(const WorkspaceEdit &WE) { llvm::json::Object Result; if (WE.changes) { diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index e88c804692568..65366a7f1307b 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1004,6 +1004,17 @@ struct CodeActionParams { }; bool fromJSON(const llvm::json::Value &, CodeActionParams &, llvm::json::Path); +struct PrepareRenameResult { + /// The range of the string to rename. + Range range; + /// A placeholder text of the string content to be renamed. + /// + /// This is useful to populate the rename field with an Objective-C selector + /// name (eg. `performAction:with:`) when renaming Objective-C methods. + std::string placeholder; +}; +llvm::json::Value toJSON(const PrepareRenameResult &WE); + /// The edit should either provide changes or documentChanges. If the client /// can handle versioned document edits and if documentChanges are present, /// the latter are preferred over changes. diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp index 835038423fdf3..0081a69357b02 100644 --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -625,16 +625,16 @@ llvm::StringMap collectIdentifiers(llvm::StringRef Content, return Identifiers; } -std::vector collectIdentifierRanges(llvm::StringRef Identifier, - llvm::StringRef Content, - const LangOptions &LangOpts) { +std::vector +collectIdentifierRanges(llvm::StringRef Identifier, + const syntax::UnexpandedTokenBuffer &Tokens) { std::vector Ranges; - lex(Content, LangOpts, - [&](const syntax::Token &Tok, const SourceManager &SM) { - if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier) - return; - Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM))); - }); + const SourceManager &SM = Tokens.sourceManager(); + for (const syntax::Token &Tok : Tokens.tokens()) { + if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier) + continue; + Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM))); + } return Ranges; } diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h index a1bb44c176120..f60683fc4f21c 100644 --- a/clang-tools-extra/clangd/SourceCode.h +++ b/clang-tools-extra/clangd/SourceCode.h @@ -217,9 +217,9 @@ llvm::StringMap collectIdentifiers(llvm::StringRef Content, const format::FormatStyle &Style); /// Collects all ranges of the given identifier in the source code. -std::vector collectIdentifierRanges(llvm::StringRef Identifier, - llvm::StringRef Content, - const LangOptions &LangOpts); +std::vector +collectIdentifierRanges(llvm::StringRef Identifier, + const syntax::UnexpandedTokenBuffer &Tokens); /// Collects words from the source code. /// Unlike collectIdentifiers: diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index bf838e53f2a21..24bd3c426b3e8 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -173,9 +173,20 @@ std::optional indexableRelation(const index::SymbolRelation &R) { bool isSpelled(SourceLocation Loc, const NamedDecl &ND) { auto Name = ND.getDeclName(); const auto NameKind = Name.getNameKind(); - if (NameKind != DeclarationName::Identifier && - NameKind != DeclarationName::CXXConstructorName) + bool PrefixComparison; + switch (NameKind) { + case DeclarationName::Identifier: + case DeclarationName::CXXConstructorName: + case DeclarationName::ObjCZeroArgSelector: + PrefixComparison = false; + break; + case DeclarationName::ObjCOneArgSelector: + case DeclarationName::ObjCMultiArgSelector: + PrefixComparison = true; + break; + default: return false; + } const auto &AST = ND.getASTContext(); const auto &SM = AST.getSourceManager(); const auto &LO = AST.getLangOpts(); @@ -183,7 +194,17 @@ bool isSpelled(SourceLocation Loc, const NamedDecl &ND) { if (clang::Lexer::getRawToken(Loc, Tok, SM, LO)) return false; auto StrName = Name.getAsString(); - return clang::Lexer::getSpelling(Tok, SM, LO) == StrName; + std::string LexerSpelling = clang::Lexer::getSpelling(Tok, SM, LO); + if (PrefixComparison) { + // The lexer spelling at the source location is only the first label of an + // Objective-C selector, eg. if `StrName` is `performAction:with:`, then the + // token at the requested location is `performAction`. Re-building the + // entire selector from the lexer is too complicated here, so just perform + // a prefix comparison. + return StringRef(StrName).starts_with(LexerSpelling); + } else { + return StrName == LexerSpelling; + } } } // namespace diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 11f9e4627af76..f639cbc9b4b12 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -26,6 +26,8 @@ #include "clang/Basic/CharInfo.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Tooling/Refactoring/Rename/RenamingAction.h" +#include "clang/Tooling/Refactoring/Rename/SymbolName.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" @@ -40,6 +42,8 @@ namespace clang { namespace clangd { namespace { +using tooling::SymbolName; + std::optional filePath(const SymbolLocation &Loc, llvm::StringRef HintFilePath) { if (!Loc) @@ -517,22 +521,27 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) { // Check if we can rename the given RenameDecl into NewName. // Return details if the rename would produce a conflict. std::optional checkName(const NamedDecl &RenameDecl, - llvm::StringRef NewName) { + const SymbolName &NewName) { trace::Span Tracer("CheckName"); static constexpr trace::Metric InvalidNameMetric( "rename_name_invalid", trace::Metric::Counter, "invalid_kind"); auto &ASTCtx = RenameDecl.getASTContext(); + auto Identifier = NewName.getSinglePiece(); + if (!Identifier) { + return std::nullopt; + } std::optional Result; - if (isKeyword(NewName, ASTCtx.getLangOpts())) - Result = InvalidName{InvalidName::Keywords, NewName.str()}; - else if (!mayBeValidIdentifier(NewName)) - Result = InvalidName{InvalidName::BadIdentifier, NewName.str()}; - else { + if (isKeyword(*Identifier, ASTCtx.getLangOpts())) { + Result = InvalidName{InvalidName::Keywords, *Identifier}; + } else if (!mayBeValidIdentifier(*Identifier)) { + Result = InvalidName{InvalidName::BadIdentifier, *Identifier}; + } else { // Name conflict detection. // Function conflicts are subtle (overloading), so ignore them. if (RenameDecl.getKind() != Decl::Function && RenameDecl.getKind() != Decl::CXXMethod) { - if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName)) + if (auto *Conflict = + lookupSiblingWithName(ASTCtx, RenameDecl, *Identifier)) Result = InvalidName{ InvalidName::Conflict, Conflict->getLocation().printToString(ASTCtx.getSourceManager())}; @@ -546,7 +555,7 @@ std::optional checkName(const NamedDecl &RenameDecl, // AST-based rename, it renames all occurrences in the main file. llvm::Expected renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl, - llvm::StringRef NewName) { + const SymbolName &OldName, const SymbolName &NewName) { trace::Span Tracer("RenameWithinFile"); const SourceManager &SM = AST.getSourceManager(); @@ -569,9 +578,30 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl, // } if (!isInsideMainFile(RenameLoc, SM)) continue; - if (auto Err = FilteredChanges.add(tooling::Replacement( - SM, CharSourceRange::getTokenRange(RenameLoc), NewName))) - return std::move(Err); + if (std::optional Identifier = NewName.getSinglePiece()) { + tooling::Replacement NewReplacement( + SM, CharSourceRange::getTokenRange(RenameLoc), *Identifier); + if (auto Err = FilteredChanges.add(NewReplacement)) { + return std::move(Err); + } + continue; + } + SmallVector PieceLocations; + llvm::Error Error = findObjCSymbolSelectorPieces( + AST.getTokens().expandedTokens(), SM, RenameLoc, OldName, + tooling::ObjCSymbolSelectorKind::Unknown, PieceLocations); + if (Error) { + // Ignore the error. We simply skip over all selectors that didn't match. + consumeError(std::move(Error)); + continue; + } + for (auto [Location, NewPiece] : + llvm::zip_equal(PieceLocations, NewName.getNamePieces())) { + tooling::Replacement NewReplacement( + SM, CharSourceRange::getTokenRange(Location), NewPiece); + if (auto Err = FilteredChanges.add(NewReplacement)) + return std::move(Err); + } } return FilteredChanges; } @@ -664,8 +694,9 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl, // there is no dirty buffer. llvm::Expected renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, - llvm::StringRef NewName, const SymbolIndex &Index, - size_t MaxLimitFiles, llvm::vfs::FileSystem &FS) { + SymbolName OldName, SymbolName NewName, + const SymbolIndex &Index, size_t MaxLimitFiles, + llvm::vfs::FileSystem &FS) { trace::Span Tracer("RenameOutsideFile"); auto AffectedFiles = findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index, MaxLimitFiles); @@ -683,10 +714,11 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, } auto AffectedFileCode = (*ExpBuffer)->getBuffer(); - auto RenameRanges = - adjustRenameRanges(AffectedFileCode, RenameDecl.getNameAsString(), - std::move(FileAndOccurrences.second), - RenameDecl.getASTContext().getLangOpts()); + syntax::UnexpandedTokenBuffer Tokens( + AffectedFileCode, RenameDecl.getASTContext().getLangOpts()); + std::optional> RenameRanges = + adjustRenameRanges(Tokens, OldName.getNamePieces().front(), + std::move(FileAndOccurrences.second)); if (!RenameRanges) { // Our heuristics fails to adjust rename ranges to the current state of // the file, it is most likely the index is stale, so we give up the @@ -695,8 +727,8 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, "(the index may be stale)", FilePath); } - auto RenameEdit = - buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, NewName); + auto RenameEdit = buildRenameEdit(FilePath, AffectedFileCode, *RenameRanges, + OldName, NewName, Tokens); if (!RenameEdit) return error("failed to rename in file {0}: {1}", FilePath, RenameEdit.takeError()); @@ -779,12 +811,25 @@ llvm::Expected rename(const RenameInputs &RInputs) { if (DeclsUnderCursor.size() > 1) return makeError(ReasonToReject::AmbiguousSymbol); const auto &RenameDecl = **DeclsUnderCursor.begin(); - const auto *ID = RenameDecl.getIdentifier(); - if (!ID) + DeclarationName Name = RenameDecl.getDeclName(); + if (!Name) return makeError(ReasonToReject::UnsupportedSymbol); - if (ID->getName() == RInputs.NewName) + SymbolName OldSymbolName(Name); + SymbolName NewSymbolName; + if (RInputs.NewName) { + NewSymbolName = SymbolName(*RInputs.NewName, AST.getLangOpts()); + } else { + // If no new name is given, we are perfoming a pseudo rename for the + // prepareRename request to check if the rename is possible. Construct a + // new symbol name that has as many name pieces as the old name and is thus + // a valid new name. + std::vector NewNamePieces = OldSymbolName.getNamePieces(); + NewNamePieces[0] += "__clangd_rename_placeholder"; + NewSymbolName = SymbolName(NewNamePieces); + } + if (OldSymbolName == NewSymbolName) return makeError(ReasonToReject::SameName); - auto Invalid = checkName(RenameDecl, RInputs.NewName); + auto Invalid = checkName(RenameDecl, NewSymbolName); if (Invalid) return makeError(std::move(*Invalid)); @@ -802,7 +847,8 @@ llvm::Expected rename(const RenameInputs &RInputs) { // To make cross-file rename work for local symbol, we use a hybrid solution: // - run AST-based rename on the main file; // - run index-based rename on other affected files; - auto MainFileRenameEdit = renameWithinFile(AST, RenameDecl, RInputs.NewName); + auto MainFileRenameEdit = + renameWithinFile(AST, RenameDecl, OldSymbolName, NewSymbolName); if (!MainFileRenameEdit) return MainFileRenameEdit.takeError(); @@ -827,6 +873,7 @@ llvm::Expected rename(const RenameInputs &RInputs) { } RenameResult Result; Result.Target = CurrentIdentifier; + Result.OldName = RenameDecl.getNameAsString(); Edit MainFileEdits = Edit(MainFileCode, std::move(*MainFileRenameEdit)); for (const TextEdit &TE : MainFileEdits.asTextEdits()) Result.LocalChanges.push_back(TE.range); @@ -847,7 +894,8 @@ llvm::Expected rename(const RenameInputs &RInputs) { } auto OtherFilesEdits = renameOutsideFile( - RenameDecl, RInputs.MainFilePath, RInputs.NewName, *RInputs.Index, + RenameDecl, RInputs.MainFilePath, OldSymbolName, NewSymbolName, + *RInputs.Index, Opts.LimitFiles == 0 ? std::numeric_limits::max() : Opts.LimitFiles, *RInputs.FS); @@ -860,10 +908,11 @@ llvm::Expected rename(const RenameInputs &RInputs) { return Result; } -llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, - llvm::StringRef InitialCode, - std::vector Occurrences, - llvm::StringRef NewName) { +llvm::Expected +buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode, + std::vector Occurrences, SymbolName OldName, + SymbolName NewName, + const syntax::UnexpandedTokenBuffer &Tokens) { trace::Span Tracer("BuildRenameEdit"); SPAN_ATTACH(Tracer, "file_path", AbsFilePath); SPAN_ATTACH(Tracer, "rename_occurrences", @@ -904,12 +953,36 @@ llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, OccurrencesOffsets.push_back({*StartOffset, *EndOffset}); } + const SourceManager &SM = Tokens.sourceManager(); + tooling::Replacements RenameEdit; for (const auto &R : OccurrencesOffsets) { - auto ByteLength = R.second - R.first; - if (auto Err = RenameEdit.add( - tooling::Replacement(AbsFilePath, R.first, ByteLength, NewName))) - return std::move(Err); + if (std::optional Identifier = NewName.getSinglePiece()) { + auto ByteLength = R.second - R.first; + if (auto Err = RenameEdit.add(tooling::Replacement( + AbsFilePath, R.first, ByteLength, *Identifier))) + return std::move(Err); + } else { + SmallVector PieceLocations; + llvm::Error Error = findObjCSymbolSelectorPieces( + Tokens.tokens(), SM, + SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(R.first), + OldName, tooling::ObjCSymbolSelectorKind::Unknown, PieceLocations); + if (Error) { + // Ignore the error. We simply skip over all selectors that didn't + // match. + consumeError(std::move(Error)); + continue; + } + assert(PieceLocations.size() == NewName.getNamePieces().size()); + for (auto [Location, NewPiece] : + llvm::zip_equal(PieceLocations, NewName.getNamePieces())) { + tooling::Replacement NewReplacement( + SM, CharSourceRange::getTokenRange(Location), NewPiece); + if (auto Err = RenameEdit.add(NewReplacement)) + return std::move(Err); + } + } } return Edit(InitialCode, std::move(RenameEdit)); } @@ -928,13 +1001,13 @@ llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, // were inserted). If such a "near miss" is found, the rename is still // possible std::optional> -adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier, - std::vector Indexed, const LangOptions &LangOpts) { +adjustRenameRanges(const syntax::UnexpandedTokenBuffer &Tokens, + llvm::StringRef Identifier, std::vector Indexed) { trace::Span Tracer("AdjustRenameRanges"); assert(!Indexed.empty()); assert(llvm::is_sorted(Indexed)); - std::vector Lexed = - collectIdentifierRanges(Identifier, DraftCode, LangOpts); + + std::vector Lexed = collectIdentifierRanges(Identifier, Tokens); 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 91728ba59e5d8..5b43875bf56a3 100644 --- a/clang-tools-extra/clangd/refactor/Rename.h +++ b/clang-tools-extra/clangd/refactor/Rename.h @@ -12,6 +12,7 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang/Basic/LangOptions.h" +#include "clang/Tooling/Refactoring/Rename/SymbolName.h" #include "llvm/Support/Error.h" #include @@ -34,8 +35,12 @@ struct RenameOptions { }; struct RenameInputs { - Position Pos; // the position triggering the rename - llvm::StringRef NewName; + /// The position triggering the rename + Position Pos; + + /// The new name to give to the symbol or `nullopt` to perform a fake rename + /// that checks if rename is possible. + std::optional NewName; ParsedAST &AST; llvm::StringRef MainFilePath; @@ -51,12 +56,14 @@ struct RenameInputs { }; struct RenameResult { - // The range of the symbol that the user can attempt to rename. + /// The range of the symbol that the user can attempt to rename. Range Target; - // Rename occurrences for the current main file. + /// The current name of the declaration at the cursor. + std::string OldName; + /// Rename occurrences for the current main file. std::vector LocalChanges; - // Complete edits for the rename, including LocalChanges. - // If the full set of changes is unknown, this field is empty. + /// Complete edits for the rename, including LocalChanges. + /// If the full set of changes is unknown, this field is empty. FileEdits GlobalChanges; }; @@ -66,13 +73,19 @@ struct RenameResult { llvm::Expected rename(const RenameInputs &RInputs); /// Generates rename edits that replaces all given occurrences with the -/// NewName. +/// `NewName`. +/// +/// `OldName` and `Tokens` are used to to find the argument labels of +/// Objective-C selectors. +/// /// Exposed for testing only. +/// /// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges. -llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, - llvm::StringRef InitialCode, - std::vector Occurrences, - llvm::StringRef NewName); +llvm::Expected +buildRenameEdit(llvm::StringRef AbsFilePath, llvm::StringRef InitialCode, + std::vector Occurrences, tooling::SymbolName OldName, + tooling::SymbolName NewName, + const syntax::UnexpandedTokenBuffer &Tokens); /// Adjusts indexed occurrences to match the current state of the file. /// @@ -85,8 +98,8 @@ llvm::Expected buildRenameEdit(llvm::StringRef AbsFilePath, /// occurrence has the same length). /// REQUIRED: Indexed is sorted. std::optional> -adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier, - std::vector Indexed, const LangOptions &LangOpts); +adjustRenameRanges(const syntax::UnexpandedTokenBuffer &Tokens, + llvm::StringRef Identifier, std::vector Indexed); /// 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/test/rename.test b/clang-tools-extra/clangd/test/rename.test index 527b4263443a7..0dc1a103002b2 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 9cbf59684fbc1..ff64e1764513e 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -876,6 +876,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 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 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 +1077,12 @@ 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}, {R"cpp( void foo(int); void foo(char); @@ -1137,7 +1288,7 @@ TEST(RenameTest, Renameable) { int [[V^ar]]; } )cpp", - nullptr, !HeaderFile}, + nullptr, !HeaderFile}, }; for (const auto& Case : Cases) { @@ -1325,6 +1476,7 @@ TEST(RenameTest, PrepareRename) { /*NewName=*/std::nullopt, {}); // Verify that for multi-file rename, we only return main-file occurrences. ASSERT_TRUE(bool(Results)) << Results.takeError(); + ASSERT_EQ(Results->OldName, "func"); // We don't know the result is complete in prepareRename (passing a nullptr // index internally), so GlobalChanges should be empty. EXPECT_TRUE(Results->GlobalChanges.empty()); @@ -1356,6 +1508,38 @@ TEST(RenameTest, PrepareRename) { } } +TEST(RenameTest, PrepareRenameObjC) { + Annotations Input(R"cpp( + @interface Foo + - (int)performA^ction:(int)action w^ith:(int)value; + @end + @implementation Foo + - (int)performA^ction:(int)action w^ith:(int)value { + return [self ^performAction^:action ^w^ith^:value]; + } + @end + )cpp"); + std::string Path = testPath("foo.m"); + MockFS FS; + FS.Files[Path] = std::string(Input.code()); + + auto ServerOpts = ClangdServer::optsForTest(); + ServerOpts.BuildDynamicSymbolIndex = true; + + trace::TestTracer Tracer; + MockCompilationDatabase CDB; + CDB.ExtraClangFlags = {"-xobjective-c"}; + ClangdServer Server(CDB, FS, ServerOpts); + runAddDocument(Server, Path, Input.code()); + + for (Position Point : Input.points()) { + auto Results = runPrepareRename(Server, Path, Point, + /*NewName=*/std::nullopt, {}); + ASSERT_TRUE(bool(Results)) << Results.takeError(); + ASSERT_EQ(Results->OldName, "performAction:with:"); + } +} + TEST(CrossFileRenameTests, DirtyBuffer) { Annotations FooCode("class [[Foo]] {};"); std::string FooPath = testPath("foo.cc"); @@ -1759,6 +1943,146 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) { } } +TEST(CrossFileRenameTests, ObjC) { + MockCompilationDatabase CDB; + CDB.ExtraClangFlags = {"-xobjective-c"}; + // rename is runnning on all "^" points in FooH. + struct Case { + llvm::StringRef FooH; + llvm::StringRef FooM; + llvm::StringRef NewName; + llvm::StringRef ExpectedFooH; + llvm::StringRef ExpectedFooM; + }; + Case Cases[] = { + // --- Zero arg selector + { + // Input + R"cpp( + @interface Foo + - (int)performA^ction; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performAction { + [self performAction]; + } + @end + )cpp", + // New name + "performNewAction", + // Expected + R"cpp( + @interface Foo + - (int)performNewAction; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performNewAction { + [self performNewAction]; + } + @end + )cpp", + }, + // --- Single arg selector + { + // Input + R"cpp( + @interface Foo + - (int)performA^ction:(int)action; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performAction:(int)action { + [self performAction:action]; + } + @end + )cpp", + // New name + "performNewAction:", + // Expected + R"cpp( + @interface Foo + - (int)performNewAction:(int)action; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performNewAction:(int)action { + [self performNewAction:action]; + } + @end + )cpp", + }, + // --- Multi arg selector + { + // Input + R"cpp( + @interface Foo + - (int)performA^ction:(int)action with:(int)value; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performAction:(int)action with:(int)value { + [self performAction:action with:value]; + } + @end + )cpp", + // New name + "performNewAction:by:", + // Expected + R"cpp( + @interface Foo + - (int)performNewAction:(int)action by:(int)value; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performNewAction:(int)action by:(int)value { + [self performNewAction:action by:value]; + } + @end + )cpp", + } + }; + + trace::TestTracer Tracer; + for (const auto &T : Cases) { + SCOPED_TRACE(T.FooH); + Annotations FooH(T.FooH); + Annotations FooM(T.FooM); + std::string FooHPath = testPath("foo.h"); + std::string FooMPath = testPath("foo.m"); + + MockFS FS; + FS.Files[FooHPath] = std::string(FooH.code()); + FS.Files[FooMPath] = std::string(FooM.code()); + + auto ServerOpts = ClangdServer::optsForTest(); + ServerOpts.BuildDynamicSymbolIndex = true; + ClangdServer Server(CDB, FS, ServerOpts); + + // Add all files to clangd server to make sure the dynamic index has been + // built. + runAddDocument(Server, FooHPath, FooH.code()); + runAddDocument(Server, FooMPath, FooM.code()); + + for (const auto &RenamePos : FooH.points()) { + EXPECT_THAT(Tracer.takeMetric("rename_files"), SizeIs(0)); + auto FileEditsList = + llvm::cantFail(runRename(Server, FooHPath, RenamePos, T.NewName, {})); + EXPECT_THAT(Tracer.takeMetric("rename_files"), ElementsAre(2)); + EXPECT_THAT(applyEdits(std::move(FileEditsList.GlobalChanges)), + UnorderedElementsAre(Pair(Eq(FooHPath), Eq(T.ExpectedFooH)), + Pair(Eq(FooMPath), Eq(T.ExpectedFooM)))); + } + } +} + TEST(CrossFileRenameTests, CrossFileOnLocalSymbol) { // cross-file rename should work for function-local symbols, even there is no // index provided. @@ -1779,7 +2103,13 @@ TEST(CrossFileRenameTests, BuildRenameEdits) { auto LSPRange = Code.range(); llvm::StringRef FilePath = "/test/TestTU.cpp"; llvm::StringRef NewName = "abc"; - auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, NewName); + tooling::SymbolName NewSymbolName(NewName, /*IsObjectiveCSelector=*/false); + tooling::SymbolName OldSymbolName("😂", /*IsObjectiveCSelector=*/false); + + syntax::UnexpandedTokenBuffer Tokens(Code.code(), LangOptions()); + + auto Edit = buildRenameEdit(FilePath, Code.code(), {LSPRange}, OldSymbolName, + NewSymbolName, Tokens); ASSERT_TRUE(bool(Edit)) << Edit.takeError(); ASSERT_EQ(1UL, Edit->Replacements.size()); EXPECT_EQ(FilePath, Edit->Replacements.begin()->getFilePath()); @@ -1787,7 +2117,8 @@ 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}, OldSymbolName, + NewSymbolName, Tokens); EXPECT_FALSE(Edit); EXPECT_THAT(llvm::toString(Edit.takeError()), testing::HasSubstr("fail to convert")); @@ -1798,7 +2129,8 @@ TEST(CrossFileRenameTests, BuildRenameEdits) { [[range]] [[range]] )cpp"); - Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), NewName); + Edit = buildRenameEdit(FilePath, T.code(), T.ranges(), OldSymbolName, + NewSymbolName, Tokens); ASSERT_TRUE(bool(Edit)) << Edit.takeError(); EXPECT_EQ(applyEdits(FileEdits{{T.code(), std::move(*Edit)}}).front().second, expectedResult(T, NewName)); @@ -1855,8 +2187,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); + syntax::UnexpandedTokenBuffer Tokens(Draft.code(), LangOpts); + auto ActualRanges = + adjustRenameRanges(Tokens, "x", Annotations(T.IndexedCode).ranges()); if (!ActualRanges) EXPECT_THAT(Draft.ranges(), testing::IsEmpty()); else diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp index 08abde87df6d4..e22bd817f9a42 100644 --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -772,8 +772,8 @@ o foo2; )cpp"); LangOptions LangOpts; LangOpts.CPlusPlus = true; - EXPECT_EQ(Code.ranges(), - collectIdentifierRanges("Foo", Code.code(), LangOpts)); + syntax::UnexpandedTokenBuffer Tokens(Code.code(), LangOpts); + EXPECT_EQ(Code.ranges(), collectIdentifierRanges("Foo", Tokens)); } TEST(SourceCodeTests, isHeaderFile) { diff --git a/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h b/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h index 43a8d56e4e717..e6b38c40c8182 100644 --- a/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h +++ b/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h @@ -19,6 +19,7 @@ #include "clang/Tooling/Refactoring/RefactoringActionRules.h" #include "clang/Tooling/Refactoring/RefactoringOptions.h" #include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h" +#include "clang/Tooling/Syntax/Tokens.h" #include "llvm/Support/Error.h" namespace clang { @@ -116,6 +117,26 @@ class QualifiedRenamingAction { std::map &FileToReplaces; }; +enum class ObjCSymbolSelectorKind { + /// The rename location is an Objective-C method call, eg. `[self add: 1]`. + MessageSend, + + /// The rename location is an Objective-C method definition, eg. + /// ` - (void)add:(int)theValue` + MethodDecl, + + /// It is unknown if the renamed location is a method call or declaration. + /// + /// The selector kind is being used to improve error recovery, passing unknown + /// does not lead to correctness issues. + Unknown +}; + +llvm::Error findObjCSymbolSelectorPieces( + ArrayRef Tokens, const SourceManager &SrcMgr, + SourceLocation RenameLoc, const SymbolName &OldName, + ObjCSymbolSelectorKind Kind, SmallVectorImpl &Result); + } // end namespace tooling } // end namespace clang diff --git a/clang/include/clang/Tooling/Refactoring/Rename/SymbolName.h b/clang/include/clang/Tooling/Refactoring/Rename/SymbolName.h index 6c28d40f3679c..887ab0929445d 100644 --- a/clang/include/clang/Tooling/Refactoring/Rename/SymbolName.h +++ b/clang/include/clang/Tooling/Refactoring/Rename/SymbolName.h @@ -9,12 +9,16 @@ #ifndef LLVM_CLANG_TOOLING_REFACTORING_RENAME_SYMBOLNAME_H #define LLVM_CLANG_TOOLING_REFACTORING_RENAME_SYMBOLNAME_H +#include "clang/AST/DeclarationName.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" namespace clang { + +class LangOptions; + namespace tooling { /// A name of a symbol. @@ -27,19 +31,45 @@ namespace tooling { /// // ^~ string 0 ~~~~~ ^~ string 1 ~~~~~ /// \endcode class SymbolName { + llvm::SmallVector NamePieces; + public: - explicit SymbolName(StringRef Name) { - // While empty symbol names are valid (Objective-C selectors can have empty - // name pieces), occurrences Objective-C selectors are created using an - // array of strings instead of just one string. - assert(!Name.empty() && "Invalid symbol name!"); - this->Name.push_back(Name.str()); - } + SymbolName(); + + /// Create a new \c SymbolName with the specified pieces. + explicit SymbolName(ArrayRef NamePieces); + explicit SymbolName(ArrayRef NamePieces); + + explicit SymbolName(const DeclarationName &Name); - ArrayRef getNamePieces() const { return Name; } + /// Creates a \c SymbolName from the given string representation. + /// + /// For Objective-C symbol names, this splits a selector into multiple pieces + /// on `:`. For all other languages the name is used as the symbol name. + SymbolName(StringRef Name, bool IsObjectiveCSelector); + SymbolName(StringRef Name, const LangOptions &LangOpts); -private: - llvm::SmallVector Name; + ArrayRef 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 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 SymbolName &Other) const { + return NamePieces == Other.NamePieces; + } }; } // end namespace tooling diff --git a/clang/include/clang/Tooling/Syntax/Tokens.h b/clang/include/clang/Tooling/Syntax/Tokens.h index b1bdefed7c97f..f765788d4e857 100644 --- a/clang/include/clang/Tooling/Syntax/Tokens.h +++ b/clang/include/clang/Tooling/Syntax/Tokens.h @@ -145,6 +145,19 @@ class Token { /// For debugging purposes. Equivalent to a call to Token::str(). llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Token &T); +/// A list of tokens as lexed from the input file, without expanding +/// preprocessor macros. +class UnexpandedTokenBuffer { + std::vector Tokens; + std::unique_ptr SrcMgr; + +public: + UnexpandedTokenBuffer(StringRef Code, const LangOptions &LangOpts); + + ArrayRef tokens() const { return Tokens; } + const SourceManager &sourceManager() const { return SrcMgr->get(); } +}; + /// A list of tokens obtained by preprocessing a text buffer and operations to /// map between the expanded and spelled tokens, i.e. TokenBuffer has /// information about two token streams: diff --git a/clang/lib/Tooling/Refactoring/CMakeLists.txt b/clang/lib/Tooling/Refactoring/CMakeLists.txt index d3077be8810aa..f78f64ea2ef64 100644 --- a/clang/lib/Tooling/Refactoring/CMakeLists.txt +++ b/clang/lib/Tooling/Refactoring/CMakeLists.txt @@ -13,6 +13,7 @@ add_clang_library(clangToolingRefactoring Rename/USRFinder.cpp Rename/USRFindingAction.cpp Rename/USRLocFinder.cpp + SymbolName.cpp LINK_LIBS clangAST @@ -23,6 +24,7 @@ add_clang_library(clangToolingRefactoring clangLex clangRewrite clangToolingCore + clangToolingSyntax DEPENDS omp_gen diff --git a/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp b/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp index 72598601d47d6..20802d3a5f101 100644 --- a/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp +++ b/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp @@ -82,7 +82,7 @@ RenameOccurrences::createSourceReplacements(RefactoringRuleContext &Context) { if (!Occurrences) return Occurrences.takeError(); // FIXME: Verify that the new name is valid. - SymbolName Name(NewName); + SymbolName Name(NewName, /*IsObjectiveCSelector=*/false); return createRenameReplacements( *Occurrences, Context.getASTContext().getSourceManager(), Name); } @@ -219,7 +219,7 @@ class RenamingASTConsumer : public ASTConsumer { } // FIXME: Support multi-piece names. // FIXME: better error handling (propagate error out). - SymbolName NewNameRef(NewName); + SymbolName NewNameRef(NewName, /*IsObjectiveCSelector=*/false); Expected> Change = createRenameReplacements(Occurrences, SourceMgr, NewNameRef); if (!Change) { @@ -275,5 +275,99 @@ std::unique_ptr QualifiedRenamingAction::newASTConsumer() { return std::make_unique(NewNames, USRList, FileToReplaces); } +static bool isMatchingSelectorName(const syntax::Token &Tok, + const syntax::Token &Next, + StringRef NamePiece, + const SourceManager &SrcMgr) { + if (NamePiece.empty()) + return Tok.kind() == tok::colon; + return (Tok.kind() == tok::identifier || Tok.kind() == tok::raw_identifier) && + Next.kind() == tok::colon && Tok.text(SrcMgr) == NamePiece; +} + +Error findObjCSymbolSelectorPieces(ArrayRef AllTokens, + const SourceManager &SM, + SourceLocation RenameLoc, + const SymbolName &OldName, + ObjCSymbolSelectorKind Kind, + SmallVectorImpl &Result) { + ArrayRef Tokens = + AllTokens.drop_while([RenameLoc](syntax::Token Tok) -> bool { + return Tok.location() != RenameLoc; + }); + assert(!Tokens.empty() && "no tokens"); + assert(OldName.getNamePieces()[0].empty() || + Tokens[0].text(SM) == OldName.getNamePieces()[0]); + assert(OldName.getNamePieces().size() > 1); + + Result.push_back(Tokens[0].location()); + + // 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. + unsigned SquareCount = 0; + unsigned ParenCount = 0; + unsigned BraceCount = 0; + + // Start looking for the next selector piece. + unsigned Last = Tokens.size() - 1; + // Skip the ':' or any other token after the first selector piece token. + for (unsigned Index = OldName.getNamePieces()[0].empty() ? 1 : 2; + Index < Last; ++Index) { + const auto &Tok = Tokens[Index]; + + bool NoScoping = SquareCount == 0 && BraceCount == 0 && ParenCount == 0; + if (NoScoping && + isMatchingSelectorName(Tok, Tokens[Index + 1], + OldName.getNamePieces()[Result.size()], SM)) { + if (!OldName.getNamePieces()[Result.size()].empty()) { + // Skip the ':' after the name. This ensures that it won't match a + // follow-up selector piece with an empty name. + ++Index; + } + Result.push_back(Tok.location()); + // All the selector pieces have been found. + if (Result.size() == OldName.getNamePieces().size()) + return Error::success(); + } else if (Tok.kind() == tok::r_square) { + // Stop scanning at the end of the message send. + // Also account for spurious ']' in blocks or lambdas. + if (Kind == ObjCSymbolSelectorKind::MessageSend && !SquareCount && + !BraceCount) + break; + if (SquareCount) + --SquareCount; + } else if (Tok.kind() == tok::l_square) { + ++SquareCount; + } else if (Tok.kind() == tok::l_paren) { + ++ParenCount; + } else if (Tok.kind() == tok::r_paren) { + if (!ParenCount) + break; + --ParenCount; + } else if (Tok.kind() == tok::l_brace) { + // Stop scanning at the start of the of the method's body. + // Also account for any spurious blocks inside argument parameter types + // or parameter attributes. + if (Kind == ObjCSymbolSelectorKind::MethodDecl && !BraceCount && + !ParenCount) + break; + ++BraceCount; + } else if (Tok.kind() == tok::r_brace) { + if (!BraceCount) + break; + --BraceCount; + } + // Stop scanning at the end of the method's declaration. + if (Kind == ObjCSymbolSelectorKind::MethodDecl && NoScoping && + (Tok.kind() == tok::semi || Tok.kind() == tok::minus || + Tok.kind() == tok::plus)) + break; + } + return llvm::make_error( + "failed to find all selector pieces in the source code", + inconvertibleErrorCode()); +} + } // end namespace tooling } // end namespace clang diff --git a/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp b/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp index c18f9290471fe..43e48f24caa9e 100644 --- a/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp +++ b/clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp @@ -60,8 +60,8 @@ class USRLocFindingASTVisitor const ASTContext &Context) : RecursiveSymbolVisitor(Context.getSourceManager(), Context.getLangOpts()), - USRSet(USRs.begin(), USRs.end()), PrevName(PrevName), Context(Context) { - } + USRSet(USRs.begin(), USRs.end()), + PrevName(PrevName, /*IsObjectiveCSelector=*/false), Context(Context) {} bool visitSymbolOccurrence(const NamedDecl *ND, ArrayRef NameRanges) { diff --git a/clang/lib/Tooling/Refactoring/SymbolName.cpp b/clang/lib/Tooling/Refactoring/SymbolName.cpp new file mode 100644 index 0000000000000..896a6cf09a3a9 --- /dev/null +++ b/clang/lib/Tooling/Refactoring/SymbolName.cpp @@ -0,0 +1,70 @@ +//===--- SymbolName.cpp - Clang refactoring library -----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Tooling/Refactoring/Rename/SymbolName.h" +#include "clang/Basic/LangOptions.h" +#include "llvm/Support/raw_ostream.h" + +namespace clang { +namespace tooling { + +SymbolName::SymbolName() : NamePieces({}) {} + +SymbolName::SymbolName(const DeclarationName &DeclName) + : SymbolName(DeclName.getAsString(), + /*IsObjectiveCSelector=*/DeclName.getNameKind() == + DeclarationName::NameKind::ObjCMultiArgSelector || + DeclName.getNameKind() == + DeclarationName::NameKind::ObjCOneArgSelector) {} + +SymbolName::SymbolName(StringRef Name, const LangOptions &LangOpts) + : SymbolName(Name, LangOpts.ObjC) {} + +SymbolName::SymbolName(StringRef Name, bool IsObjectiveCSelector) { + if (!IsObjectiveCSelector) { + NamePieces.push_back(Name.str()); + return; + } + // Decompose an Objective-C selector name into multiple strings. + do { + auto StringAndName = Name.split(':'); + NamePieces.push_back(StringAndName.first.str()); + Name = StringAndName.second; + } while (!Name.empty()); +} + +SymbolName::SymbolName(ArrayRef NamePieces) { + for (const auto &Piece : NamePieces) + this->NamePieces.push_back(Piece.str()); +} + +SymbolName::SymbolName(ArrayRef NamePieces) { + for (const auto &Piece : NamePieces) + this->NamePieces.push_back(Piece); +} + +std::optional SymbolName::getSinglePiece() const { + if (getNamePieces().size() == 1) { + return NamePieces.front(); + } + return std::nullopt; +} + +std::string SymbolName::getAsString() const { + std::string Result; + llvm::raw_string_ostream OS(Result); + this->print(OS); + return Result; +} + +void SymbolName::print(raw_ostream &OS) const { + llvm::interleave(NamePieces, OS, ":"); +} + +} // end namespace tooling +} // end namespace clang diff --git a/clang/lib/Tooling/Syntax/Tokens.cpp b/clang/lib/Tooling/Syntax/Tokens.cpp index 8d32c45a4a70c..1f7a2cb4f499b 100644 --- a/clang/lib/Tooling/Syntax/Tokens.cpp +++ b/clang/lib/Tooling/Syntax/Tokens.cpp @@ -225,6 +225,13 @@ llvm::StringRef FileRange::text(const SourceManager &SM) const { return Text.substr(Begin, length()); } +UnexpandedTokenBuffer::UnexpandedTokenBuffer(StringRef Code, + const LangOptions &LangOpts) { + SrcMgr = std::make_unique("mock_file_name.cpp", Code); + Tokens = syntax::tokenize(sourceManager().getMainFileID(), sourceManager(), + LangOpts); +} + void TokenBuffer::indexExpandedTokens() { // No-op if the index is already created. if (!ExpandedTokIndex.empty()) diff --git a/clang/tools/clang-refactor/CMakeLists.txt b/clang/tools/clang-refactor/CMakeLists.txt index a21d84d5385b4..092740be15f01 100644 --- a/clang/tools/clang-refactor/CMakeLists.txt +++ b/clang/tools/clang-refactor/CMakeLists.txt @@ -20,4 +20,5 @@ clang_target_link_libraries(clang-refactor clangTooling clangToolingCore clangToolingRefactoring + clangToolingSyntax ) diff --git a/clang/tools/clang-rename/CMakeLists.txt b/clang/tools/clang-rename/CMakeLists.txt index f4c4e520520d9..2f79f6ce4f3ff 100644 --- a/clang/tools/clang-rename/CMakeLists.txt +++ b/clang/tools/clang-rename/CMakeLists.txt @@ -16,6 +16,7 @@ clang_target_link_libraries(clang-rename clangTooling clangToolingCore clangToolingRefactoring + clangToolingSyntax ) install(FILES clang-rename.py diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt index 1cfc46eb1a52f..f400986be797a 100644 --- a/clang/tools/libclang/CMakeLists.txt +++ b/clang/tools/libclang/CMakeLists.txt @@ -69,6 +69,7 @@ set(LIBS clangSema clangSerialization clangTooling + clangToolingSyntax ) if (CLANG_ENABLE_ARCMT) diff --git a/clang/unittests/Tooling/RefactoringActionRulesTest.cpp b/clang/unittests/Tooling/RefactoringActionRulesTest.cpp index cdd1faf0e38d4..c405ea02f90f6 100644 --- a/clang/unittests/Tooling/RefactoringActionRulesTest.cpp +++ b/clang/unittests/Tooling/RefactoringActionRulesTest.cpp @@ -211,9 +211,9 @@ TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) { Expected findSymbolOccurrences(RefactoringRuleContext &) override { SymbolOccurrences Occurrences; - Occurrences.push_back(SymbolOccurrence(SymbolName("test"), - SymbolOccurrence::MatchingSymbol, - Selection.getBegin())); + Occurrences.push_back(SymbolOccurrence( + SymbolName("test", /*IsObjectiveCSelector=*/false), + SymbolOccurrence::MatchingSymbol, Selection.getBegin())); return std::move(Occurrences); } };