Skip to content

Commit a4340ff

Browse files
author
Nathan Hawes
committed
[sourcekitd][Refactoring] Fix renaming of var decls in fallthrough case statements
We weren't renaming all occurrences of 'x' in the cases like the below: case .first(let x), .second(let x): print("foo \(x)") fallthrough case .third(let x): print("bar \(x)") We would previously only rename occurrences within the case statement the query was made in (ignoring fallthroughs) and for cases with multiple patterns (as in the first case above) we would only rename the occurrence in the first pattern.
1 parent 096871e commit a4340ff

16 files changed

+115
-17
lines changed

‎lib/AST/USRGeneration.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ static bool shouldUseObjCUSR(const Decl *D) {
167167
llvm::Expected<std::string>
168168
swift::USRGenerationRequest::evaluate(Evaluator &evaluator,
169169
const ValueDecl *D) const {
170+
if (auto *VD = dyn_cast<VarDecl>(D))
171+
D = VD->getCanonicalVarDecl();
172+
170173
if (!D->hasName() && !isa<ParamDecl>(D) && !isa<AccessorDecl>(D))
171174
return std::string(); // Ignore.
172175
if (D->getModuleContext()->isBuiltinModule())

‎lib/Index/Index.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,12 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
516516
}
517517

518518
bool shouldIndex(ValueDecl *D, bool IsRef) const {
519+
if (D->isImplicit() && isa<VarDecl>(D) && IsRef) {
520+
// Bypass the implicit VarDecls introduced in CaseStmt bodies by using the
521+
// canonical VarDecl for these checks instead.
522+
D = cast<VarDecl>(D)->getCanonicalVarDecl();
523+
}
524+
519525
if (D->isImplicit() && !isa<ConstructorDecl>(D))
520526
return false;
521527

@@ -1151,6 +1157,11 @@ bool IndexSwiftASTWalker::reportImplicitConformance(ValueDecl *witness, ValueDec
11511157
bool IndexSwiftASTWalker::initIndexSymbol(ValueDecl *D, SourceLoc Loc,
11521158
bool IsRef, IndexSymbol &Info) {
11531159
assert(D);
1160+
if (auto *VD = dyn_cast<VarDecl>(D)) {
1161+
// Always base the symbol information on the canonical VarDecl
1162+
D = VD->getCanonicalVarDecl();
1163+
}
1164+
11541165
Info.decl = D;
11551166
Info.symInfo = getSymbolInfoForDecl(D);
11561167
if (Info.symInfo.Kind == SymbolKind::Unknown)

‎lib/Parse/ParseStmt.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,6 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
12041204

12051205
for (auto VD : repeatedDecls) {
12061206
VD->setHasNonPatternBindingInit();
1207-
VD->setImplicit();
12081207
}
12091208

12101209
// Parse the optional 'where' guard, with this particular pattern's bound

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2450,6 +2450,20 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
24502450
auto *var = elt.first;
24512451
unsigned access = elt.second;
24522452

2453+
if (auto *CS = dyn_cast_or_null<CaseStmt>(var->getRecursiveParentPatternStmt())) {
2454+
// Only diagnose VarDecls from the first CaseLabelItem in CaseStmts, as
2455+
// the remaining items must match it anyway.
2456+
auto CaseItems = CS->getCaseLabelItems();
2457+
if (!CaseItems.empty()) {
2458+
bool InFirstCaseLabelItem = false;
2459+
CaseItems.front().getPattern()->forEachVariable([&](VarDecl *D) {
2460+
InFirstCaseLabelItem |= var == D;
2461+
});
2462+
if (!InFirstCaseLabelItem)
2463+
continue;
2464+
}
2465+
}
2466+
24532467
// If this is a 'let' value, any stores to it are actually initializations,
24542468
// not mutations.
24552469
auto isWrittenLet = false;

‎test/Index/local.swift

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,47 @@ func foo(a: Int, b: Int, c: Int) {
3535
let _ = LocalEnum.foo(x: LocalStruct())
3636
// LOCAL: [[@LINE-1]]:13 | enum(local)/Swift | LocalEnum | [[LocalEnum_USR]] | Ref,RelCont | rel: 1
3737
// CHECK-NOT: [[@LINE-2]]:13 | enum(local)/Swift | LocalEnum | [[LocalEnum_USR]] | Ref,RelCont | rel: 1
38-
// LOCAL: [[@LINE-3]]:23 | enumerator(local)/Swift | foo(x:) | [[LocalEnum_foo_USR]] | Ref,RelCont | rel: 1
38+
// LOCAL: [[@LINE-3]]:23 | enumerator(local)/Swift | foo(x:) | [[LocalEnum_foo_USR]] | Ref,RelCont | rel: 1
3939
// CHECK-NOT: [[@LINE-4]]:23 | enumerator(local)/Swift | foo(x:) | [[LocalEnum_foo_USR]] | Ref,RelCont | rel: 1
4040
// LOCAL: [[@LINE-5]]:30 | struct(local)/Swift | LocalStruct | [[LocalStruct_USR]] | Ref,RelCont | rel: 1
4141
// CHECK-NOT: [[@LINE-6]]:30 | struct(local)/Swift | LocalStruct | [[LocalStruct_USR]] | Ref,RelCont | rel: 1
4242

4343
}
44+
45+
func bar(arg: Int?) {
46+
switch arg {
47+
case let .some(x) where x == 0:
48+
// LOCAL: [[@LINE-1]]:18 | variable(local)/Swift | x | [[x_USR:.*]] | Def,RelChild | rel: 1
49+
// LOCAL: [[@LINE-2]]:27 | variable(local)/Swift | x | [[x_USR]] | Ref,Read,RelCont | rel: 1
50+
// CHECK-NOT: [[@LINE-3]]:18 | variable(local)/Swift | x | [[x_USR:.*]] | Def,RelChild | rel: 1
51+
// CHECK-NOT: [[@LINE-4]]:27 | variable(local)/Swift | x | [[x_USR]] | Ref,Read,RelCont | rel: 1
52+
print(x)
53+
// LOCAL: [[@LINE-1]]:11 | variable(local)/Swift | x | [[x_USR]] | Ref,Read,RelCont | rel: 1
54+
// CHECK-NOT: [[@LINE-2]]:11 | variable(local)/Swift | x | [[x_USR]] | Ref,Read,RelCont | rel: 1
55+
56+
case let .some(x) where x == 1,
57+
// LOCAL: [[@LINE-1]]:18 | variable(local)/Swift | x | [[x2_USR:.*]] | Def,RelChild | rel: 1
58+
// LOCAL: [[@LINE-2]]:27 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
59+
// CHECK-NOT: [[@LINE-3]]:18 | variable(local)/Swift | x | [[x2_USR:.*]] | Def,RelChild | rel: 1
60+
// CHECK-NOT: [[@LINE-4]]:27 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
61+
let .some(x) where x == 2:
62+
// LOCAL: [[@LINE-1]]:18 | variable(local)/Swift | x | [[x2_USR]] | Def,RelChild | rel: 1
63+
// LOCAL: [[@LINE-2]]:27 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
64+
// CHECK-NOT: [[@LINE-3]]:18 | variable(local)/Swift | x | [[x2_USR]] | Def,RelChild | rel: 1
65+
// CHECK-NOT: [[@LINE-4]]:27 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
66+
print(x)
67+
// LOCAL: [[@LINE-1]]:11 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
68+
// CHECK-NOT: [[@LINE-2]]:11 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
69+
fallthrough
70+
case let .some(x) where x == 3:
71+
// LOCAL: [[@LINE-1]]:18 | variable(local)/Swift | x | [[x2_USR]] | Def,RelChild | rel: 1
72+
// LOCAL: [[@LINE-2]]:27 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
73+
// CHECK-NOT: [[@LINE-1]]:18 | variable(local)/Swift | x | [[x2_USR]] | Def,RelChild | rel: 1
74+
// CHECK-NOT: [[@LINE-2]]:27 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
75+
print(x)
76+
// LOCAL: [[@LINE-1]]:11 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
77+
// CHECK-NOT: [[@LINE-1]]:11 | variable(local)/Swift | x | [[x2_USR]] | Ref,Read,RelCont | rel: 1
78+
default:
79+
break
80+
}
81+
}

‎test/refactoring/rename/Outputs/local/casebind_1.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(xRenamed) where xRenamed == 0:
2222
print(xRenamed)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/casebind_2.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(xRenamed) where xRenamed == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(xRenamed) where xRenamed == 2:
25+
print(xRenamed)
26+
fallthrough
27+
case let .some(xRenamed) where xRenamed == 3:
2528
print(xRenamed)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/catch_1.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/catch_2.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/ifbind_1.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/ifbind_2.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/localvar_1.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/localvar_2.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/param_1.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/Outputs/local/param_2.swift.expected

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break

‎test/refactoring/rename/local.swift

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ func test3(arg: Int?) {
2121
case let .some(x) where x == 0:
2222
print(x)
2323
case let .some(x) where x == 1,
24-
let .some(x) where x == 2: // FIXME: This 'x' in '.some(x)' isn't properly renamed in 'casebind_2' case.
24+
let .some(x) where x == 2:
25+
print(x)
26+
fallthrough
27+
case let .some(x) where x == 3:
2528
print(x)
2629
default:
2730
break
@@ -57,11 +60,11 @@ func test5(_ x: Int) {
5760
// RUN: %refactor -rename -source-filename %s -pos=25:11 -new-name="xRenamed" >> %t.result/casebind_2.swift
5861
// RUN: diff -u %S/Outputs/local/casebind_1.swift.expected %t.result/casebind_1.swift
5962
// RUN: diff -u %S/Outputs/local/casebind_2.swift.expected %t.result/casebind_2.swift
60-
// RUN: %refactor -rename -source-filename %s -pos=35:15 -new-name="xRenamed" >> %t.result/catch_1.swift
61-
// RUN: %refactor -rename -source-filename %s -pos=38:11 -new-name="xRenamed" >> %t.result/catch_2.swift
63+
// RUN: %refactor -rename -source-filename %s -pos=38:15 -new-name="xRenamed" >> %t.result/catch_1.swift
64+
// RUN: %refactor -rename -source-filename %s -pos=41:11 -new-name="xRenamed" >> %t.result/catch_2.swift
6265
// RUN: diff -u %S/Outputs/local/catch_1.swift.expected %t.result/catch_1.swift
6366
// RUN: diff -u %S/Outputs/local/catch_2.swift.expected %t.result/catch_2.swift
64-
// RUN: %refactor -rename -source-filename %s -pos=42:14 -new-name="xRenamed" >> %t.result/param_1.swift
65-
// RUN: %refactor -rename -source-filename %s -pos=44:9 -new-name="xRenamed" >> %t.result/param_2.swift
67+
// RUN: %refactor -rename -source-filename %s -pos=45:14 -new-name="xRenamed" >> %t.result/param_1.swift
68+
// RUN: %refactor -rename -source-filename %s -pos=47:9 -new-name="xRenamed" >> %t.result/param_2.swift
6669
// RUN: diff -u %S/Outputs/local/param_1.swift.expected %t.result/param_1.swift
6770
// RUN: diff -u %S/Outputs/local/param_2.swift.expected %t.result/param_2.swift

0 commit comments

Comments
 (0)