diff --git a/lib/Refactoring/SyntacticRenameRangeDetails.cpp b/lib/Refactoring/SyntacticRenameRangeDetails.cpp index cb4e9d056dfd9..6e29b1f3f599c 100644 --- a/lib/Refactoring/SyntacticRenameRangeDetails.cpp +++ b/lib/Refactoring/SyntacticRenameRangeDetails.cpp @@ -470,8 +470,13 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges( isCallSite = true; break; case RenameLocUsage::Definition: - // All function definitions have argument labels that should be renamed. - handleLabels = true; + // Don't rename labels of operators. There is a mismatch between the + // indexer reporting all labels as `_` even if they are spelled as e.g. + // `x: Int` in the function declaration. Since the labels only appear + // on the operator declaration and in no calls, just don't rename them for + // now. + // For all other (normal) function declarations, always rename the labels. + handleLabels = !Lexer::isOperator(Old.base()); isCallSite = false; break; case RenameLocUsage::Reference: diff --git a/test/SourceKit/RelatedIdents/operator.swift b/test/SourceKit/RelatedIdents/operator.swift new file mode 100644 index 0000000000000..93a7e8253571a --- /dev/null +++ b/test/SourceKit/RelatedIdents/operator.swift @@ -0,0 +1,17 @@ +// RUN: %empty-directory(%t) +// RUN: split-file --leading-lines %s %t + +//--- a.swift + +struct Foo {} +// RUN: %sourcekitd-test -req=related-idents -pos=%(line + 1):6 %t/a.swift -- %t/a.swift | %FileCheck %s +func +(x: Foo, y: Foo) {} +Foo() + Foo() + +//--- dummy.swift + +// CHECK: START RANGES +// CHECK: 8:6 - 1 - source.syntacticrename.definition +// CHECK: 9:7 - 1 - source.syntacticrename.call +// CHECK: END RANGES +// CHECK: NAME: +(_:_:) \ No newline at end of file diff --git a/test/SourceKit/RelatedIdents/related_idents.swift b/test/SourceKit/RelatedIdents/related_idents.swift index f2c3d03b92664..d8310c3a1a09e 100644 --- a/test/SourceKit/RelatedIdents/related_idents.swift +++ b/test/SourceKit/RelatedIdents/related_idents.swift @@ -109,11 +109,12 @@ func ifLet(test: Int?) { // RUN: %sourcekitd-test -req=related-idents -pos=6:17 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK1 %s // CHECK1: START RANGES -// CHECK1-NEXT: 1:7 - 2 -// CHECK1-NEXT: 6:11 - 2 -// CHECK1-NEXT: 6:16 - 2 -// CHECK1-NEXT: 9:11 - 2 +// CHECK1-NEXT: 1:7 - 2 - source.syntacticrename.definition +// CHECK1-NEXT: 6:11 - 2 - source.syntacticrename.reference +// CHECK1-NEXT: 6:16 - 2 - source.syntacticrename.reference +// CHECK1-NEXT: 9:11 - 2 - source.syntacticrename.reference // CHECK1-NEXT: END RANGES +// CHECK1: NAME: C1 // RUN: %sourcekitd-test -req=related-idents -pos=5:9 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK2 %s // CHECK2: START RANGES @@ -196,6 +197,7 @@ func ifLet(test: Int?) { // CHECK11-NEXT: 74:1 - 13 // CHECK11-NEXT: 75:1 - 11 // CHECK11-NEXT: 76:1 - 11 +// CHECK11: NAME: escapedName(x:) // RUN: %sourcekitd-test -req=related-idents -pos=79:7 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK12 %s diff --git a/test/refactoring/SyntacticRename/Outputs/functions/infix-operator.swift.expected b/test/refactoring/SyntacticRename/Outputs/functions/infix-operator.swift.expected index f53754298a37e..759f847932ad0 100644 --- a/test/refactoring/SyntacticRename/Outputs/functions/infix-operator.swift.expected +++ b/test/refactoring/SyntacticRename/Outputs/functions/infix-operator.swift.expected @@ -28,7 +28,7 @@ class AStruct { return {a in a}; } - static func /*infix-operator:def*/+ (left: AStruct, right: AStruct) -> AStruct { + static func /*infix-operator:def*/+ (left: AStruct, right: AStruct) -> AStruct { return AStruct() } diff --git a/test/refactoring/SyntacticRename/Outputs/functions/prefix-operator.swift.expected b/test/refactoring/SyntacticRename/Outputs/functions/prefix-operator.swift.expected index a3efa428c284f..a618031c77d6e 100644 --- a/test/refactoring/SyntacticRename/Outputs/functions/prefix-operator.swift.expected +++ b/test/refactoring/SyntacticRename/Outputs/functions/prefix-operator.swift.expected @@ -32,7 +32,7 @@ class AStruct { return AStruct() } - static prefix func /*prefix-operator:def*/- (struct: AStruct) -> AStruct { + static prefix func /*prefix-operator:def*/- (struct: AStruct) -> AStruct { return AStruct() } } diff --git a/test/refactoring/SyntacticRename/operator.swift b/test/refactoring/SyntacticRename/operator.swift new file mode 100644 index 0000000000000..f7d9d56251ade --- /dev/null +++ b/test/refactoring/SyntacticRename/operator.swift @@ -0,0 +1,17 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t +// RUN: %refactor -find-rename-ranges -source-filename %t/input.swift -pos="test" -old-name "+(x:y:)" -new-name "-(x:y:)" > %t/output.txt +// RUN: diff -u %t/expected.swift %t/output.txt + +//--- input.swift + +struct Foo {} +func /*test:def*/+(x: Foo, y: Foo) {} +Foo() /*test:ref*/+ Foo() + +//--- expected.swift + +struct Foo {} +func /*test:def*/+(x: Foo, y: Foo) {} +Foo() /*test:ref*/+ Foo() + diff --git a/tools/SourceKit/include/SourceKit/Core/LangSupport.h b/tools/SourceKit/include/SourceKit/Core/LangSupport.h index 0f84cc9a74d79..cfe5762410249 100644 --- a/tools/SourceKit/include/SourceKit/Core/LangSupport.h +++ b/tools/SourceKit/include/SourceKit/Core/LangSupport.h @@ -786,6 +786,19 @@ struct SemanticRefactoringInfo { struct RelatedIdentInfo { unsigned Offset; unsigned Length; + swift::ide::RenameLocUsage Usage; +}; + +/// Result of `findRelatedIdentifiersInFile`. +struct RelatedIdentsResult { + SmallVector RelatedIdents; + std::string OldName; + + RelatedIdentsResult(SmallVector RelatedIdents, + std::string OldName) + : RelatedIdents(RelatedIdents), OldName(OldName) {} + + static RelatedIdentsResult empty() { return RelatedIdentsResult({}, ""); } }; /// Represent one branch of an if config. @@ -1149,11 +1162,20 @@ class LangSupport { SourceKitCancellationToken CancellationToken, std::function &)> Receiver) = 0; + /// - Parameters: + /// - IncludeNonEditableBaseNames: If `true` also return results if the + /// referenced declaration is an initializer or subscript. This is + /// intended if the related identifiers response is used for rename, which + /// allows renaming parameter labels of these declaration. + /// If the function's base name should be highlighted, this should be + /// `false` because e.g. highlighting a subscript with + /// `IncludeNonEditableBaseNames = true` would return the locations of all + /// `[` that call that subscript. virtual void findRelatedIdentifiersInFile( StringRef PrimaryFilePath, StringRef InputBufferName, unsigned Offset, - bool CancelOnSubsequentRequest, ArrayRef Args, - SourceKitCancellationToken CancellationToken, - std::function> &)> + bool IncludeNonEditableBaseNames, bool CancelOnSubsequentRequest, + ArrayRef Args, SourceKitCancellationToken CancellationToken, + std::function &)> Receiver) = 0; virtual void findActiveRegionsInFile( diff --git a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h index 5f7aa1efe96c9..fa14c5233b7f2 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h +++ b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h @@ -682,10 +682,10 @@ class SwiftLangSupport : public LangSupport { void findRelatedIdentifiersInFile( StringRef PrimaryFilePath, StringRef InputBufferName, unsigned Offset, - bool CancelOnSubsequentRequest, ArrayRef Args, - SourceKitCancellationToken CancellationToken, - std::function> &)> - Receiver) override; + bool IncludeNonEditableBaseNames, bool CancelOnSubsequentRequest, + ArrayRef Args, SourceKitCancellationToken CancellationToken, + std::function &)> Receiver) + override; void findActiveRegionsInFile( StringRef PrimaryFilePath, StringRef InputBufferName, diff --git a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp index 0eb1d30bb3069..75ef7e7fa21c2 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp @@ -2491,126 +2491,139 @@ SwiftLangSupport::findUSRRange(StringRef DocumentName, StringRef USR) { void SwiftLangSupport::findRelatedIdentifiersInFile( StringRef PrimaryFilePath, StringRef InputBufferName, unsigned Offset, - bool CancelOnSubsequentRequest, ArrayRef Args, - SourceKitCancellationToken CancellationToken, - std::function> &)> - Receiver) { + bool IncludeNonEditableBaseNames, bool CancelOnSubsequentRequest, + ArrayRef Args, SourceKitCancellationToken CancellationToken, + std::function &)> Receiver) { std::string Error; SwiftInvocationRef Invok = ASTMgr->getTypecheckInvocation(Args, PrimaryFilePath, Error); if (!Invok) { LOG_WARN_FUNC("failed to create an ASTInvocation: " << Error); - Receiver(RequestResult>::fromError(Error)); + Receiver(RequestResult::fromError(Error)); return; } class RelatedIdConsumer : public SwiftASTConsumer { std::string InputFile; unsigned Offset; - std::function> &)> - Receiver; + bool IncludeNonEditableBaseNames; + std::function &)> Receiver; SwiftInvocationRef Invok; +#if SWIFT_BUILD_SWIFT_SYNTAX + // FIXME: Don't silently eat errors here. + RelatedIdentsResult getRelatedIdents(SourceFile *SrcFile, + CompilerInstance &CompInst) { + unsigned BufferID = SrcFile->getBufferID().value(); + SourceLoc Loc = Lexer::getLocForStartOfToken(CompInst.getSourceMgr(), + BufferID, Offset); + if (Loc.isInvalid()) + return RelatedIdentsResult::empty(); + + SourceManager &SrcMgr = CompInst.getASTContext().SourceMgr; + + ResolvedCursorInfoPtr CursorInfo = + evaluateOrDefault(CompInst.getASTContext().evaluator, + CursorInfoRequest{CursorInfoOwner(SrcFile, Loc)}, + new ResolvedCursorInfo()); + auto ValueRefCursorInfo = + dyn_cast(CursorInfo); + if (!ValueRefCursorInfo) + return RelatedIdentsResult::empty(); + if (ValueRefCursorInfo->isKeywordArgument()) + return RelatedIdentsResult::empty(); + + ValueDecl *VD = ValueRefCursorInfo->typeOrValue(); + if (!VD) + return RelatedIdentsResult::empty(); // This was a module reference. + + // Only accept pointing to an identifier. + if (!IncludeNonEditableBaseNames && !ValueRefCursorInfo->isRef() && + (isa(VD) || isa(VD) || + isa(VD))) + return RelatedIdentsResult::empty(); + + llvm::Optional Info = getRenameInfo(CursorInfo); + + if (!Info) { + return RelatedIdentsResult::empty(); + } + + RenameLocs Locs = localRenameLocs(SrcFile, Info->VD); + + std::string OldName = Locs.getLocations().front().OldName.str(); +#ifndef NDEBUG + for (auto loc : Locs.getLocations()) { + assert(loc.OldName == OldName && + "Found related identfiers with different names?"); + } +#endif + + // Ignore any errors produced by `resolveRenameLocations` since, if some + // symbol failed to resolve, we still want to return all the other + // symbols. This makes related idents more fault-tolerant. + DiagnosticEngine Diags(SrcMgr); + + std::vector ResolvedLocs = resolveRenameLocations( + Locs.getLocations(), /*NewName=*/StringRef(), *SrcFile, Diags); + + SmallVector Ranges; + assert(ResolvedLocs.size() == Locs.getLocations().size()); + for (auto [RenameLoc, ResolvedLoc] : + llvm::zip_equal(Locs.getLocations(), ResolvedLocs)) { + if (ResolvedLoc.range.isInvalid()) { + continue; + } + unsigned Offset = + SrcMgr.getLocOffsetInBuffer(ResolvedLoc.range.getStart(), BufferID); + Ranges.push_back( + {Offset, ResolvedLoc.range.getByteLength(), RenameLoc.Usage}); + } + + return RelatedIdentsResult(Ranges, OldName); + } +#endif + public: RelatedIdConsumer( - StringRef InputFile, unsigned Offset, - std::function> &)> + StringRef InputFile, unsigned Offset, bool IncludeNonEditableBaseNames, + std::function &)> Receiver, SwiftInvocationRef Invok) : InputFile(InputFile.str()), Offset(Offset), + IncludeNonEditableBaseNames(IncludeNonEditableBaseNames), Receiver(std::move(Receiver)), Invok(Invok) {} - // FIXME: Don't silently eat errors here. void handlePrimaryAST(ASTUnitRef AstUnit) override { - using ResultType = RequestResult>; + using ResultType = RequestResult; #if !SWIFT_BUILD_SWIFT_SYNTAX - Receiver( - ResultType::fromError("relatedidents is not supported because " - "sourcekitd was built without swift-syntax")); + ResultType::fromError( + "relatedidents is not supported because sourcekitd was built without " + "swift-syntax"); return; #else auto &CompInst = AstUnit->getCompilerInstance(); auto *SrcFile = retrieveInputFile(InputFile, CompInst); if (!SrcFile) { - Receiver(RequestResult>::fromError( + Receiver(RequestResult::fromError( "Unable to find input file")); return; } - SmallVector Ranges; - - auto Action = [&]() { - unsigned BufferID = SrcFile->getBufferID().value(); - SourceLoc Loc = - Lexer::getLocForStartOfToken(CompInst.getSourceMgr(), BufferID, Offset); - if (Loc.isInvalid()) - return; - - SourceManager &SrcMgr = CompInst.getASTContext().SourceMgr; - - ResolvedCursorInfoPtr CursorInfo = - evaluateOrDefault(CompInst.getASTContext().evaluator, - CursorInfoRequest{CursorInfoOwner(SrcFile, Loc)}, - new ResolvedCursorInfo()); - auto ValueRefCursorInfo = - dyn_cast(CursorInfo); - if (!ValueRefCursorInfo) - return; - if (ValueRefCursorInfo->isKeywordArgument()) - return; - - ValueDecl *VD = ValueRefCursorInfo->typeOrValue(); - if (!VD) - return; // This was a module reference. - - // Only accept pointing to an identifier. - if (!ValueRefCursorInfo->isRef() && - (isa(VD) || isa(VD) || - isa(VD))) - return; - if (VD->isOperator()) - return; - - llvm::Optional Info = getRenameInfo(CursorInfo); - - if (!Info) { - return; - } - - RenameLocs Locs = localRenameLocs(SrcFile, Info->VD); - - // Ignore any errors produced by `resolveRenameLocations` since, if some - // symbol failed to resolve, we still want to return all the other - // symbols. This makes related idents more fault-tolerant. - DiagnosticEngine Diags(SrcMgr); - - std::vector ResolvedLocs = resolveRenameLocations( - Locs.getLocations(), /*NewName=*/StringRef(), *SrcFile, Diags); - - assert(ResolvedLocs.size() == Locs.getLocations().size()); - for (auto ResolvedLoc : ResolvedLocs) { - if (ResolvedLoc.range.isInvalid()) { - continue; - } - unsigned Offset = SrcMgr.getLocOffsetInBuffer( - ResolvedLoc.range.getStart(), BufferID); - Ranges.push_back({Offset, ResolvedLoc.range.getByteLength()}); - } - }; - Action(); - Receiver(ResultType::fromResult(Ranges)); + RelatedIdentsResult Result = getRelatedIdents(SrcFile, CompInst); + Receiver(ResultType::fromResult(Result)); #endif } void cancelled() override { - Receiver(RequestResult>::cancelled()); + Receiver(RequestResult::cancelled()); } void failed(StringRef Error) override { LOG_WARN_FUNC("related idents failed: " << Error); - Receiver(RequestResult>::fromError(Error)); + Receiver(RequestResult::fromError(Error)); } static CaseStmt *getCaseStmtOfCanonicalVar(Decl *D) { @@ -2624,8 +2637,8 @@ void SwiftLangSupport::findRelatedIdentifiersInFile( } }; - auto Consumer = std::make_shared(InputBufferName, Offset, - Receiver, Invok); + auto Consumer = std::make_shared( + InputBufferName, Offset, IncludeNonEditableBaseNames, Receiver, Invok); /// FIXME: When request cancellation is implemented and Xcode adopts it, /// don't use 'OncePerASTToken'. static const char OncePerASTToken = 0; diff --git a/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp b/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp index 5d8785df64730..c98499d4e05c0 100644 --- a/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp +++ b/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp @@ -2456,10 +2456,13 @@ static void printRelatedIdents(sourcekitd_variant_t Info, StringRef Filename, sourcekitd_variant_t Range = sourcekitd_variant_array_get_value(Res, i); int64_t Offset = sourcekitd_variant_dictionary_get_int64(Range, KeyOffset); int64_t Length = sourcekitd_variant_dictionary_get_int64(Range, KeyLength); + auto Usage = sourcekitd_variant_dictionary_get_uid(Range, KeyNameType); auto LineCol = resolveToLineCol(Offset, Filename, VFSFiles); - OS << LineCol.first << ':' << LineCol.second << " - " << Length << '\n'; + OS << LineCol.first << ':' << LineCol.second << " - " << Length << " - " + << sourcekitd_uid_get_string_ptr(Usage) << '\n'; } OS << "END RANGES\n"; + OS << "NAME: " << sourcekitd_variant_dictionary_get_string(Info, KeyName); } static void printActiveRegions(sourcekitd_variant_t Info, StringRef Filename, diff --git a/tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp b/tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp index 5ca26ea6aa076..75893b781d90e 100644 --- a/tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp +++ b/tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp @@ -212,6 +212,7 @@ static void reportNameInfo(const RequestResult &Result, Res static void findRelatedIdents(StringRef PrimaryFilePath, StringRef InputBufferName, int64_t Offset, + bool IncludeNonEditableBaseNames, bool CancelOnSubsequentRequest, ArrayRef Args, SourceKitCancellationToken CancellationToken, @@ -1787,14 +1788,18 @@ handleRequestRelatedIdents(const RequestDict &Req, if (Req.getInt64(KeyOffset, Offset, /*isOptional=*/false)) return Rec(createErrorRequestInvalid("missing 'key.offset'")); + int64_t IncludeNonEditableBaseNames = 0; + Req.getInt64(KeyIncludeNonEditableBaseNames, IncludeNonEditableBaseNames, + /*isOptional=*/true); + // For backwards compatibility, the default is 1. int64_t CancelOnSubsequentRequest = 1; Req.getInt64(KeyCancelOnSubsequentRequest, CancelOnSubsequentRequest, /*isOptional=*/true); - return findRelatedIdents(*PrimaryFilePath, InputBufferName, Offset, - CancelOnSubsequentRequest, Args, CancellationToken, - Rec); + return findRelatedIdents( + *PrimaryFilePath, InputBufferName, Offset, IncludeNonEditableBaseNames, + CancelOnSubsequentRequest, Args, CancellationToken, Rec); }); } @@ -2858,31 +2863,47 @@ reportVariableTypeInfo(const RequestResult &Result, // FindRelatedIdents //===----------------------------------------------------------------------===// +static sourcekitd_uid_t renameLocUsageUID(swift::ide::RenameLocUsage Usage) { + switch (Usage) { + case swift::ide::RenameLocUsage::Definition: + return KindDefinition; + case swift::ide::RenameLocUsage::Reference: + return KindReference; + case swift::ide::RenameLocUsage::Call: + return KindCall; + case swift::ide::RenameLocUsage::Unknown: + return KindUnknown; + } +} + static void findRelatedIdents(StringRef PrimaryFilePath, StringRef InputBufferName, int64_t Offset, + bool IncludeNonEditableBaseNames, bool CancelOnSubsequentRequest, ArrayRef Args, SourceKitCancellationToken CancellationToken, ResponseReceiver Rec) { LangSupport &Lang = getGlobalContext().getSwiftLangSupport(); Lang.findRelatedIdentifiersInFile( - PrimaryFilePath, InputBufferName, Offset, CancelOnSubsequentRequest, Args, - CancellationToken, - [Rec](const RequestResult> &Result) { + PrimaryFilePath, InputBufferName, Offset, IncludeNonEditableBaseNames, + CancelOnSubsequentRequest, Args, CancellationToken, + [Rec](const RequestResult &Result) { if (Result.isCancelled()) return Rec(createErrorRequestCancelled()); if (Result.isError()) return Rec(createErrorRequestFailed(Result.getError())); - const ArrayRef &Info = Result.value(); + const RelatedIdentsResult &Info = Result.value(); ResponseBuilder RespBuilder; auto Arr = RespBuilder.getDictionary().setArray(KeyResults); - for (auto R : Info) { + for (auto R : Info.RelatedIdents) { auto Elem = Arr.appendDictionary(); Elem.set(KeyOffset, R.Offset); Elem.set(KeyLength, R.Length); + Elem.set(KeyNameType, renameLocUsageUID(R.Usage)); } + RespBuilder.getDictionary().set(KeyName, Info.OldName); Rec(RespBuilder.createResponse()); }); diff --git a/utils/gyb_sourcekit_support/UIDs.py b/utils/gyb_sourcekit_support/UIDs.py index b03391e7739b9..10de7600bedb2 100644 --- a/utils/gyb_sourcekit_support/UIDs.py +++ b/utils/gyb_sourcekit_support/UIDs.py @@ -146,6 +146,7 @@ def __init__(self, internal_name, external_name): KEY('Simplified', 'key.simplified'), KEY('RangeContent', 'key.rangecontent'), KEY('CancelOnSubsequentRequest', 'key.cancel_on_subsequent_request'), + KEY('IncludeNonEditableBaseNames', 'key.include_non_editable_base_names'), KEY('RenameLocations', 'key.renamelocations'), KEY('Locations', 'key.locations'), KEY('NameType', 'key.nametype'),