Skip to content

Commit 9c54d79

Browse files
xedinxedin
authored
Merge pull request #37897 from mininny/control-flow-suggestion-for-optional
[Diagnostics] Offer 'is' replacement for unused 'if let' expression with optional operand
2 parents 57bd730 + 031d132 commit 9c54d79

File tree

3 files changed

+72
-12
lines changed

3 files changed

+72
-12
lines changed

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2878,23 +2878,41 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
28782878

28792879
// If the subexpr is an "as?" cast, we can rewrite it to
28802880
// be an "is" test.
2881-
bool isIsTest = false;
2882-
if (isa<ConditionalCheckedCastExpr>(initExpr) &&
2883-
!initExpr->isImplicit()) {
2884-
noParens = isIsTest = true;
2881+
ConditionalCheckedCastExpr *CCE = nullptr;
2882+
2883+
// initExpr can be wrapped inside parens or try expressions.
2884+
if (auto ccExpr = dyn_cast<ConditionalCheckedCastExpr>(
2885+
initExpr->getValueProvidingExpr())) {
2886+
if (!ccExpr->isImplicit()) {
2887+
CCE = ccExpr;
2888+
noParens = true;
2889+
}
28852890
}
2886-
2891+
2892+
// In cases where the value is optional, the cast expr is
2893+
// wrapped inside OptionalEvaluationExpr. Unwrap it to get
2894+
// ConditionalCheckedCastExpr.
2895+
if (auto oeExpr = dyn_cast<OptionalEvaluationExpr>(
2896+
initExpr->getValueProvidingExpr())) {
2897+
if (auto ccExpr = dyn_cast<ConditionalCheckedCastExpr>(
2898+
oeExpr->getSubExpr()->getValueProvidingExpr())) {
2899+
if (!ccExpr->isImplicit()) {
2900+
CCE = ccExpr;
2901+
noParens = true;
2902+
}
2903+
}
2904+
}
2905+
28872906
auto diagIF = Diags.diagnose(var->getLoc(),
28882907
diag::pbd_never_used_stmtcond,
28892908
var->getName());
28902909
auto introducerLoc = SC->getCond()[0].getIntroducerLoc();
28912910
diagIF.fixItReplaceChars(introducerLoc,
28922911
initExpr->getStartLoc(),
28932912
&"("[noParens]);
2894-
2895-
if (isIsTest) {
2913+
2914+
if (CCE) {
28962915
// If this was an "x as? T" check, rewrite it to "x is T".
2897-
auto CCE = cast<ConditionalCheckedCastExpr>(initExpr);
28982916
diagIF.fixItReplace(SourceRange(CCE->getLoc(),
28992917
CCE->getQuestionLoc()),
29002918
"is");

‎test/decl/var/usage.swift

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,44 @@ func test(_ a : Int?, b : Any) {
330330
if let x = b as? Int { // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}} {{6-14=}} {{16-19=is}}
331331
}
332332

333-
// SR-1112
333+
// SR-14646. Special case, turn this into an 'is' test with optional value.
334+
let bb: Any? = 3
335+
if let bbb = bb as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{19-22=is}}
336+
if let bbb = (bb) as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{21-24=is}}
337+
338+
func aa() -> Any? { return 1 }
339+
if let aaa = aa() as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{21-24=is}}
340+
if let aaa = (aa()) as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{23-26=is}}
341+
342+
func bb() -> Any { return 1 }
343+
if let aaa = aa() as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{21-24=is}}
344+
if let aaa = (aa()) as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{23-26=is}}
334345

346+
347+
func throwingAA() throws -> Any? { return 1 }
348+
do {
349+
if let aaa = try! throwingAA() as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{36-39=is}}
350+
if let aaa = (try! throwingAA()) as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{38-41=is}}
351+
if let aaa = try throwingAA() as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{35-38=is}}
352+
if let aaa = (try throwingAA()) as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{37-40=is}}
353+
} catch { }
354+
if let aaa = try? throwingAA() as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{6-16=(}} {{41-41=) != nil}}
355+
if let aaa = (try? throwingAA()) as? Int {} // expected-warning {{value 'aaa' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{36-39=is}}
356+
357+
func throwingBB() throws -> Any { return 1 }
358+
do {
359+
if let bbb = try! throwingBB() as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{36-39=is}}
360+
if let bbb = (try! throwingBB()) as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{38-41=is}}
361+
if let bbb = try throwingBB() as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{35-38=is}}
362+
if let bbb = (try throwingBB()) as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{8-18=}} {{37-40=is}}
363+
} catch { }
364+
if let bbb = try? throwingBB() as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{6-16=(}} {{41-41=) != nil}}
365+
if let bbb = (try? throwingBB()) as? Int {} // expected-warning {{value 'bbb' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{36-39=is}}
366+
367+
let cc: (Any?, Any) = (1, 2)
368+
if let (cc1, cc2) = cc as? (Int, Int) {} // expected-warning {{immutable value 'cc1' was never used; consider replacing with '_' or removing it}} expected-warning {{immutable value 'cc2' was never used; consider replacing with '_' or removing it}}
369+
370+
// SR-1112
335371
let xxx: Int? = 0
336372

337373
if let yyy = xxx { } // expected-warning{{with boolean test}} {{6-16=}} {{19-19= != nil}}
@@ -356,7 +392,7 @@ let optionalString: String? = "check"
356392
if let string = optionalString {} // expected-warning {{value 'string' was defined but never used; consider replacing with boolean test}} {{4-17=}} {{31-31= != nil}}
357393

358394
let optionalAny: Any? = "check"
359-
if let string = optionalAny as? String {} // expected-warning {{value 'string' was defined but never used; consider replacing with boolean test}} {{4-17=(}} {{39-39=) != nil}}
395+
if let string = optionalAny as? String {} // expected-warning {{value 'string' was defined but never used; consider replacing with boolean test}} {{4-17=}} {{29-32=is}}
360396

361397
// Due to the complexities of global variable tracing, these will not generate warnings
362398
let unusedVariable = ""
@@ -504,3 +540,9 @@ func testVariablesBoundInPatterns() {
504540
break
505541
}
506542
}
543+
// Tests fix to SR-14646
544+
func testUselessCastWithInvalidParam(foo: Any?) -> Int {
545+
class Foo { }
546+
if let bar = foo as? Foo { return 42 } // expected-warning {{value 'bar' was defined but never used; consider replacing with boolean test}} {{6-16=}} {{20-23=is}}
547+
else { return 54 }
548+
}

‎test/stmt/if_while_var.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ if let x = foo() {
5959

6060
var opt: Int? = .none
6161

62-
if let x = opt {} // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}}
63-
if var x = opt {} // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}}
62+
if let x = opt {} // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}} {{4-12=}} {{15-15= != nil}}
63+
if var x = opt {} // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}} {{4-12=}} {{15-15= != nil}}
6464

6565
// <rdar://problem/20800015> Fix error message for invalid if-let
6666
let someInteger = 1

0 commit comments

Comments
 (0)