Skip to content

Commit bbd2848

Browse files
authored
Merge pull request swiftlang#58684 from ahoppen/pr/related-idents
[SourceKit] In related identes, report underlying var of closure capture and shorthand if let binding
2 parents ee0c228 + d0d3d42 commit bbd2848

File tree

2 files changed

+206
-16
lines changed

2 files changed

+206
-16
lines changed

test/SourceKit/RelatedIdents/related_idents.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,36 @@ func `escapedName`(`x`: Int) {}
7575
escapedName(`x`: 2)
7676
escapedName(`x`:)
7777

78+
func closureCapture() {
79+
let test = 0
80+
81+
let outerClosure = { [test] in
82+
let innerClosure = { [test] in
83+
print(test)
84+
}
85+
}
86+
}
87+
88+
func closureCaptureWithExplicitName() {
89+
let test = 0
90+
91+
let closure = { [test = test] in
92+
print(test)
93+
}
94+
}
95+
96+
func shorthandIfLet(test: Int?) {
97+
if let test {
98+
print(test)
99+
}
100+
}
101+
102+
func ifLet(test: Int?) {
103+
if let test = test {
104+
print(test)
105+
}
106+
}
107+
78108
// RUN: %sourcekitd-test -req=related-idents -pos=6:17 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK1 %s
79109
// CHECK1: START RANGES
80110
// CHECK1-NEXT: 1:7 - 2
@@ -165,3 +195,51 @@ escapedName(`x`:)
165195
// CHECK11-NEXT: 75:1 - 11
166196
// CHECK11-NEXT: 76:1 - 11
167197

198+
199+
// RUN: %sourcekitd-test -req=related-idents -pos=79:7 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK12 %s
200+
// RUN: %sourcekitd-test -req=related-idents -pos=81:25 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK12 %s
201+
// RUN: %sourcekitd-test -req=related-idents -pos=82:27 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK12 %s
202+
// RUN: %sourcekitd-test -req=related-idents -pos=83:13 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK12 %s
203+
// CHECK12: START RANGES
204+
// CHECK12-NEXT: 79:7 - 4
205+
// CHECK12-NEXT: 81:25 - 4
206+
// CHECK12-NEXT: 82:27 - 4
207+
// CHECK12-NEXT: 83:13 - 4
208+
// CHECK12-NEXT: END RANGES
209+
210+
// RUN: %sourcekitd-test -req=related-idents -pos=89:7 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK13 %s
211+
// RUN: %sourcekitd-test -req=related-idents -pos=91:27 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK13 %s
212+
// CHECK13: START RANGES
213+
// CHECK13-NEXT: 89:7 - 4
214+
// CHECK13-NEXT: 91:27 - 4
215+
// CHECK13-NEXT: END RANGES
216+
217+
// RUN: %sourcekitd-test -req=related-idents -pos=91:20 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK14 %s
218+
// RUN: %sourcekitd-test -req=related-idents -pos=92:11 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK14 %s
219+
// CHECK14: START RANGES
220+
// CHECK14-NEXT: 91:20 - 4
221+
// CHECK14-NEXT: 92:11 - 4
222+
// CHECK14-NEXT: END RANGES
223+
224+
// RUN: %sourcekitd-test -req=related-idents -pos=96:21 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK15 %s
225+
// RUN: %sourcekitd-test -req=related-idents -pos=97:10 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK15 %s
226+
// RUN: %sourcekitd-test -req=related-idents -pos=98:11 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK15 %s
227+
// CHECK15: START RANGES
228+
// CHECK15-NEXT: 96:21 - 4
229+
// CHECK15-NEXT: 97:10 - 4
230+
// CHECK15-NEXT: 98:11 - 4
231+
// CHECK15-NEXT: END RANGES
232+
233+
// RUN: %sourcekitd-test -req=related-idents -pos=102:12 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK16 %s
234+
// RUN: %sourcekitd-test -req=related-idents -pos=103:17 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK16 %s
235+
// CHECK16: START RANGES
236+
// CHECK16-NEXT: 102:12 - 4
237+
// CHECK16-NEXT: 103:17 - 4
238+
// CHECK16-NEXT: END RANGES
239+
240+
// RUN: %sourcekitd-test -req=related-idents -pos=103:10 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK17 %s
241+
// RUN: %sourcekitd-test -req=related-idents -pos=104:11 %s -- -module-name related_idents %s | %FileCheck -check-prefix=CHECK17 %s
242+
// CHECK17: START RANGES
243+
// CHECK17-NEXT: 103:10 - 4
244+
// CHECK17-NEXT: 104:11 - 4
245+
// CHECK17-NEXT: END RANGES

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 128 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2158,17 +2158,22 @@ SwiftLangSupport::findUSRRange(StringRef DocumentName, StringRef USR) {
21582158
namespace {
21592159
class RelatedIdScanner : public SourceEntityWalker {
21602160
ValueDecl *Dcl;
2161-
llvm::SmallVectorImpl<std::pair<unsigned, unsigned>> &Ranges;
2161+
llvm::SmallDenseSet<std::pair<unsigned, unsigned>, 8> &Ranges;
2162+
/// Declarations that are tied to the same name as \c Dcl and should thus also
2163+
/// be renamed if \c Dcl is renamed. Most notabliy this contains closure
2164+
/// captures like `[foo]`.
2165+
llvm::SmallVectorImpl<ValueDecl *> &RelatedDecls;
21622166
SourceManager &SourceMgr;
21632167
unsigned BufferID = -1;
21642168
bool Cancelled = false;
21652169

21662170
public:
2167-
explicit RelatedIdScanner(SourceFile &SrcFile, unsigned BufferID,
2168-
ValueDecl *D,
2169-
llvm::SmallVectorImpl<std::pair<unsigned, unsigned>> &Ranges)
2170-
: Ranges(Ranges), SourceMgr(SrcFile.getASTContext().SourceMgr),
2171-
BufferID(BufferID) {
2171+
explicit RelatedIdScanner(
2172+
SourceFile &SrcFile, unsigned BufferID, ValueDecl *D,
2173+
llvm::SmallDenseSet<std::pair<unsigned, unsigned>, 8> &Ranges,
2174+
llvm::SmallVectorImpl<ValueDecl *> &RelatedDecls)
2175+
: Ranges(Ranges), RelatedDecls(RelatedDecls),
2176+
SourceMgr(SrcFile.getASTContext().SourceMgr), BufferID(BufferID) {
21722177
if (auto *V = dyn_cast<VarDecl>(D)) {
21732178
// Always use the canonical var decl for comparison. This is so we
21742179
// pick up all occurrences of x in case statements like the below:
@@ -2190,6 +2195,80 @@ class RelatedIdScanner : public SourceEntityWalker {
21902195
}
21912196

21922197
private:
2198+
bool walkToExprPre(Expr *E) override {
2199+
if (Cancelled)
2200+
return false;
2201+
2202+
// Check if there are closure captures like `[foo]` where the caputred
2203+
// variable should also be renamed
2204+
if (auto CaptureList = dyn_cast<CaptureListExpr>(E)) {
2205+
for (auto Capture : CaptureList->getCaptureList()) {
2206+
if (Capture.PBD->getPatternList().size() != 1) {
2207+
continue;
2208+
}
2209+
auto *DRE = dyn_cast_or_null<DeclRefExpr>(Capture.PBD->getInit(0));
2210+
if (!DRE) {
2211+
continue;
2212+
}
2213+
2214+
auto DeclaredVar = Capture.getVar();
2215+
if (DeclaredVar->getLoc() != DRE->getLoc()) {
2216+
// We have a capture like `[foo]` if the declared var and the
2217+
// reference share the same location.
2218+
continue;
2219+
}
2220+
2221+
auto *ReferencedVar = dyn_cast_or_null<VarDecl>(DRE->getDecl());
2222+
if (!ReferencedVar) {
2223+
continue;
2224+
}
2225+
2226+
assert(DeclaredVar->getName() == ReferencedVar->getName());
2227+
if (DeclaredVar == Dcl) {
2228+
RelatedDecls.push_back(ReferencedVar);
2229+
} else if (ReferencedVar == Dcl) {
2230+
RelatedDecls.push_back(DeclaredVar);
2231+
}
2232+
}
2233+
}
2234+
return true;
2235+
}
2236+
2237+
bool walkToStmtPre(Stmt *S) override {
2238+
if (Cancelled)
2239+
return false;
2240+
2241+
if (auto CondStmt = dyn_cast<LabeledConditionalStmt>(S)) {
2242+
for (const StmtConditionElement &Cond : CondStmt->getCond()) {
2243+
if (Cond.getKind() != StmtConditionElement::CK_PatternBinding) {
2244+
continue;
2245+
}
2246+
auto Init = dyn_cast<DeclRefExpr>(Cond.getInitializer());
2247+
if (!Init) {
2248+
continue;
2249+
}
2250+
auto ReferencedVar = dyn_cast_or_null<VarDecl>(Init->getDecl());
2251+
if (!ReferencedVar) {
2252+
continue;
2253+
}
2254+
2255+
Cond.getPattern()->forEachVariable([&](VarDecl *DeclaredVar) {
2256+
if (DeclaredVar->getLoc() != Init->getLoc()) {
2257+
return;
2258+
}
2259+
assert(DeclaredVar->getName() == ReferencedVar->getName());
2260+
if (DeclaredVar == Dcl) {
2261+
RelatedDecls.push_back(ReferencedVar);
2262+
}
2263+
if (ReferencedVar == Dcl) {
2264+
RelatedDecls.push_back(DeclaredVar);
2265+
}
2266+
});
2267+
}
2268+
}
2269+
return true;
2270+
}
2271+
21932272
bool walkToDeclPre(Decl *D, CharSourceRange Range) override {
21942273
if (Cancelled)
21952274
return false;
@@ -2232,7 +2311,7 @@ class RelatedIdScanner : public SourceEntityWalker {
22322311

22332312
bool passId(CharSourceRange Range) {
22342313
unsigned Offset = SourceMgr.getLocOffsetInBuffer(Range.getStart(),BufferID);
2235-
Ranges.push_back({ Offset, Range.getByteLength() });
2314+
Ranges.insert({Offset, Range.getByteLength()});
22362315
return !Cancelled;
22372316
}
22382317
};
@@ -2301,21 +2380,54 @@ void SwiftLangSupport::findRelatedIdentifiersInFile(
23012380
if (VD->isOperator())
23022381
return;
23032382

2304-
RelatedIdScanner Scanner(SrcFile, BufferID, VD, Ranges);
2383+
// Record ranges in a set first so we don't record some ranges twice.
2384+
// This could happen in capture lists where e.g. `[foo]` is both the
2385+
// reference of the captured variable and the declaration of the
2386+
// variable usable in the closure.
2387+
llvm::SmallDenseSet<std::pair<unsigned, unsigned>, 8> RangesSet;
2388+
2389+
// List of decls whose ranges should be reported as related identifiers.
2390+
SmallVector<ValueDecl *, 2> Worklist;
2391+
Worklist.push_back(VD);
2392+
2393+
// Decls that we have already visited, so we don't walk circles.
2394+
SmallPtrSet<ValueDecl *, 2> VisitedDecls;
2395+
while (!Worklist.empty()) {
2396+
ValueDecl *Dcl = Worklist.back();
2397+
Worklist.pop_back();
2398+
if (!VisitedDecls.insert(Dcl).second) {
2399+
// We have already visited this decl. Don't visit it again.
2400+
continue;
2401+
}
2402+
2403+
RelatedIdScanner Scanner(SrcFile, BufferID, Dcl, RangesSet, Worklist);
23052404

2306-
if (auto *Case = getCaseStmtOfCanonicalVar(VD)) {
2307-
Scanner.walk(Case);
2308-
while ((Case = Case->getFallthroughDest().getPtrOrNull())) {
2405+
if (auto *Case = getCaseStmtOfCanonicalVar(Dcl)) {
23092406
Scanner.walk(Case);
2407+
while ((Case = Case->getFallthroughDest().getPtrOrNull())) {
2408+
Scanner.walk(Case);
2409+
}
2410+
} else if (DeclContext *LocalDC =
2411+
Dcl->getDeclContext()->getLocalContext()) {
2412+
Scanner.walk(LocalDC);
2413+
} else {
2414+
Scanner.walk(SrcFile);
23102415
}
2311-
} else if (DeclContext *LocalDC = VD->getDeclContext()->getLocalContext()) {
2312-
Scanner.walk(LocalDC);
2313-
} else {
2314-
Scanner.walk(SrcFile);
23152416
}
2417+
2418+
// Sort ranges so we get deterministic output.
2419+
Ranges.insert(Ranges.end(), RangesSet.begin(), RangesSet.end());
2420+
llvm::sort(Ranges,
2421+
[](const std::pair<unsigned, unsigned> &LHS,
2422+
const std::pair<unsigned, unsigned> &RHS) -> bool {
2423+
if (LHS.first == RHS.first) {
2424+
return LHS.second < RHS.second;
2425+
} else {
2426+
return LHS.first < RHS.first;
2427+
}
2428+
});
23162429
};
23172430
Action();
2318-
23192431
RelatedIdentsInfo Info;
23202432
Info.Ranges = Ranges;
23212433
Receiver(RequestResult<RelatedIdentsInfo>::fromResult(Info));

0 commit comments

Comments
 (0)