Produce warnings when implicit optional promotions are introduced in …

…some

common standard library operators.  This is progress towards:
<rdar://problem/27457457> [Type checker] Diagnose unsavory optional injections

but there is more work to be done here.

include/swift/AST/DiagnosticsSema.def

@@ -2214,6 +2214,13 @@ NOTE(fix_unqualified_access_top_level_multi,none,
      "use '%0' to reference the %1 in module %2",
      (StringRef, DescriptiveDeclKind, Identifier))
 
+WARNING(use_of_qq_on_non_optional_value,none,
+        "left side of nil coalescing operator '?""?' is non-optional, "
+        "so the right side is never used", ())
+WARNING(nonoptional_compare_to_nil,none,
+        "comparing non-optional value to nil always returns"
+        " %select{false|true}0", (bool))
+
 ERROR(type_of_metatype,none,
       "'.dynamicType' is not allowed after a type name", ())
 ERROR(invalid_noescape_use,none,

lib/Sema/MiscDiagnostics.cpp

@@ -88,6 +88,7 @@ static void diagSelfAssignment(TypeChecker &TC, const Expr *E) {
 ///     argument lists.
 ///   - 'self.init' and 'super.init' cannot be wrapped in a larger expression
 ///     or statement.
+///   - Warn about promotions to optional in specific syntactic forms.
 ///
 static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
                                          const DeclContext *DC,
@@ -268,6 +269,9 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
       // Check function calls, looking through implicit conversions on the
       // function and inspecting the arguments directly.
       if (auto *Call = dyn_cast<ApplyExpr>(E)) {
+        // Warn about surprising implicit optional promotions.
+        checkOptionalPromotions(Call);
+
         // Check for tuple splat.
         checkTupleSplat(Call);
 
@@ -594,6 +598,72 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E,
           .fixItInsert(DRE->getStartLoc(), namePlusDot);
       }
     }
+
+    /// Return true if this expression is an implicit promotion from T to T?.
+    static bool isImplicitPromotionToOptional(Expr *E) {
+      return E->isImplicit() && isa<InjectIntoOptionalExpr>(E);
+    }
+
+    /// Return true if this is 'nil' type checked as an Optional.  This looks
+    /// like this:
+    ///   (call_expr implicit type='Int?'
+    ///     (constructor_ref_call_expr implicit
+    ///       (declref_expr implicit decl=Optional.init(nilLiteral:)
+    static bool isTypeCheckedOptionalNil(Expr *E) {
+      auto CE = dyn_cast<CallExpr>(E->getSemanticsProvidingExpr());
+      if (!CE || !CE->isImplicit() ||
+          !CE->getType()->getAnyOptionalObjectType())
+        return false;
+
+      auto CRCE = dyn_cast<ConstructorRefCallExpr>(CE->getSemanticFn());
+      if (!CRCE || !CRCE->isImplicit()) return false;
+
+      auto DRE = dyn_cast<DeclRefExpr>(CRCE->getSemanticFn());
+
+      SmallString<32> NameBuffer;
+      auto name = DRE->getDecl()->getFullName().getString(NameBuffer);
+      return name == "init(nilLiteral:)";
+    }
+
+
+    /// Warn about surprising implicit optional promotions involving operands to
+    /// calls.  Specifically, we warn about these expressions when the 'x'
+    /// operand is implicitly promoted to optional:
+    ///
+    ///       x ?? y
+    ///       x == nil    // also !=
+    ///
+    void checkOptionalPromotions(ApplyExpr *call) {
+      auto DRE = dyn_cast<DeclRefExpr>(call->getSemanticFn());
+      auto args = dyn_cast<TupleExpr>(call->getArg());
+      if (!DRE || !DRE->getDecl()->isOperator() ||
+          !args || args->getNumElements() != 2)
+        return;
+
+      auto lhs = args->getElement(0);
+      auto rhs = args->getElement(1);
+      auto calleeName = DRE->getDecl()->getName().str();
+
+      if (calleeName == "??" && isImplicitPromotionToOptional(lhs)) {
+        TC.diagnose(DRE->getLoc(), diag::use_of_qq_on_non_optional_value)
+          .highlight(lhs->getSourceRange())
+          .fixItRemove(SourceRange(DRE->getLoc(), rhs->getEndLoc()));
+        return;
+      }
+
+      if (calleeName == "==" || calleeName == "!=") {
+        if ((isImplicitPromotionToOptional(lhs) &&
+             isTypeCheckedOptionalNil(rhs)) ||
+            (isTypeCheckedOptionalNil(lhs) &&
+             isImplicitPromotionToOptional(rhs))) {
+          TC.diagnose(DRE->getLoc(), diag::nonoptional_compare_to_nil,
+                      calleeName == "!=")
+            .highlight(lhs->getSourceRange())
+            .highlight(rhs->getSourceRange());
+          return;
+        }
+      }
+    }
   };
 
   DiagnoseWalker Walker(TC, DC, isExprStmt);

test/Constraints/diagnostics.swift

@@ -214,6 +214,7 @@ class r20201968C {
 // <rdar://problem/21459429> QoI: Poor compilation error calling assert
 func r21459429(_ a : Int) {
   assert(a != nil, "ASSERT COMPILATION ERROR")
+  // expected-warning @-1 {{comparing non-optional value to nil always returns true}}
 }
 
 
@@ -781,10 +782,10 @@ class SR1594 {
 }
 
 func nilComparison(i: Int, o: AnyObject) {
-  _ = i == nil
-  _ = nil == i
-  _ = i != nil
-  _ = nil != i
+  _ = i == nil // expected-warning {{comparing non-optional value to nil always returns false}}
+  _ = nil == i // expected-warning {{comparing non-optional value to nil always returns false}}
+  _ = i != nil // expected-warning {{comparing non-optional value to nil always returns true}}
+  _ = nil != i // expected-warning {{comparing non-optional value to nil always returns true}}
   _ = i < nil
   _ = nil < i
   _ = i <= nil

test/expr/cast/nil_value_to_optional.swift

@@ -5,8 +5,8 @@ var f = false
 
 func markUsed<T>(_ t: T) {}
 
-markUsed(t != nil)
-markUsed(f != nil)
+markUsed(t != nil) // expected-warning {{comparing non-optional value to nil always returns true}}
+markUsed(f != nil) // expected-warning {{comparing non-optional value to nil always returns true}}
 
 class C : Equatable {}
 
@@ -15,7 +15,7 @@ func == (lhs: C, rhs: C) -> Bool {
 }
 
 func test(_ c: C) {
-  if c == nil {}
+  if c == nil {}  // expected-warning {{comparing non-optional value to nil always returns false}}
 }
 
 class D {}

test/expr/expressions.swift

@@ -705,10 +705,10 @@ func invalidDictionaryLiteral() {
 //===----------------------------------------------------------------------===//
 // nil/metatype comparisons
 //===----------------------------------------------------------------------===//
-_ = Int.self == nil
-_ = nil == Int.self
-_ = Int.self != nil
-_ = nil != Int.self
+_ = Int.self == nil  // expected-warning {{comparing non-optional value to nil always returns false}}
+_ = nil == Int.self  // expected-warning {{comparing non-optional value to nil always returns false}}
+_ = Int.self != nil  // expected-warning {{comparing non-optional value to nil always returns true}}
+_ = nil != Int.self  // expected-warning {{comparing non-optional value to nil always returns true}}
 
 // <rdar://problem/19032294> Disallow postfix ? when not chaining
 func testOptionalChaining(_ a : Int?, b : Int!, c : Int??) {
@@ -737,6 +737,12 @@ func testNilCoalescePrecedence(cond: Bool, a: Int?, r: CountableClosedRange<Int>
   let r2 = (r ?? 0)...42 // not ok: expected-error {{binary operator '??' cannot be applied to operands of type 'CountableClosedRange<Int>?' and 'Int'}}
   // expected-note @-1 {{overloads for '??' exist with these partially matching parameter lists:}}
   let r3 = r ?? 0...42 // parses as the first one, not the second.
+
+
+  // <rdar://problem/27457457> [Type checker] Diagnose unsavory optional injections
+  // Accidental optional injection for ??.
+  let i = 42
+  _ = i ?? 17 // expected-warning {{left side of nil coalescing operator '??' is non-optional, so the right side is never used}} {{9-15=}}
 }
 
 // <rdar://problem/19772570> Parsing of as and ?? regressed