Merge pull request #1961 from jckarter/unused-if-let-fixit

Fix a couple of issues with the fixit for unused `if let` bindings.

Author: jckarter

lib/AST/Expr.cpp

@@ -597,7 +597,10 @@ bool Expr::canAppendCallParentheses() const {
   case ExprKind::PointerToPointer:
   case ExprKind::LValueToPointer:
   case ExprKind::ForeignObjectConversion:
-    return false;
+    // Implicit conversion nodes have no syntax of their own; defer to the
+    // subexpression.
+    return cast<ImplicitConversionExpr>(this)->getSubExpr()
+      ->canAppendCallParentheses();
 
   case ExprKind::ForcedCheckedCast:
   case ExprKind::ConditionalCheckedCast:

lib/Sema/MiscDiagnostics.cpp

@@ -1466,9 +1466,7 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
             if (auto LP = dyn_cast<VarPattern>(OSP->getSubPattern()))
               if (isa<NamedPattern>(LP->getSubPattern())) {
                 auto initExpr = SC->getCond()[0].getInitializer();
-                auto beforeExprLoc =
-                  initExpr->getStartLoc().getAdvancedLocOrInvalid(-1);
-                if (beforeExprLoc.isValid()) {
+                if (initExpr->getStartLoc().isValid()) {
                   unsigned noParens = initExpr->canAppendCallParentheses();
 
                   // If the subexpr is an "as?" cast, we can rewrite it to
@@ -1483,8 +1481,9 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
                                             diag::pbd_never_used_stmtcond,
                                             var->getName());
                   auto introducerLoc = SC->getCond()[0].getIntroducerLoc();
-                  diagIF.fixItReplace(SourceRange(introducerLoc, beforeExprLoc),
-                                      &"("[noParens]);
+                  diagIF.fixItReplaceChars(introducerLoc,
+                                           initExpr->getStartLoc(),
+                                           &"("[noParens]);
 
                   if (isIsTest) {
                     // If this was an "x as? T" check, rewrite it to "x is T".

test/decl/var/usage.swift

@@ -197,7 +197,6 @@ func testFixitsInStatementsWithPatterns(a : Int?) {
   }
 }
 
-
 // <rdar://22774938> QoI: "never used" in an "if let" should rewrite expression to use != nil
 func test(a : Int?, b : Any) {
   if true == true, let x = a {   // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{24-25=_}}
@@ -217,7 +216,19 @@ func test(a : Int?, b : Any) {
   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}}
   }
 
-  
+  // SR-1112
+
+  let xxx: Int? = 0
+
+  if let yyy = xxx { } // expected-warning{{with boolean test}} {{6-16=}} {{19-19= != nil}}
+
+  var zzz: Int? = 0
+  zzz = 1
+
+  if let yyy = zzz { } // expected-warning{{with boolean test}} {{6-16=}} {{19-19= != nil}}
+
+  if let yyy = zzz ?? xxx { } // expected-warning{{with boolean test}} {{6-16=(}} {{26-26=) != nil}}
+
 }