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