Merge pull request #29628 from owenv/condition_diags

[Diagnostics] Improve diagnostics for optional/integer as boolean condition

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -3175,7 +3175,10 @@ NOTE(note_explicitly_compare_cftypeid,none,
 
 ERROR(optional_used_as_boolean,none,
      "optional type %0 cannot be used as a boolean; "
-     "test for '!= nil' instead", (Type))
+     "test for '%select{!|=}1= nil' instead", (Type, bool))
+ERROR(integer_used_as_boolean,none,
+     "type %0 cannot be used as a boolean; "
+     "test for '%select{!|=}1= 0' instead", (Type, bool))
 
 ERROR(interpolation_missing_proto,none,
       "string interpolation requires the protocol 'ExpressibleByStringInterpolation' to be defined",

include/swift/AST/KnownIdentifiers.def

@@ -143,6 +143,7 @@ IDENTIFIER_WITH_NAME(NativeClassLayout, "_NativeClass")
 // Operators
 IDENTIFIER_WITH_NAME(MatchOperator, "~=")
 IDENTIFIER_WITH_NAME(EqualsOperator, "==")
+IDENTIFIER_WITH_NAME(NegationOperator, "!")
 IDENTIFIER_WITH_NAME(derived_enum_equals, "__derived_enum_equals")
 IDENTIFIER_WITH_NAME(derived_struct_equals, "__derived_struct_equals")
 

include/swift/AST/KnownProtocols.def

@@ -69,6 +69,7 @@ PROTOCOL_(ErrorCodeProtocol)
 PROTOCOL(OptionSet)
 PROTOCOL(CaseIterable)
 PROTOCOL(SIMDScalar)
+PROTOCOL(BinaryInteger)
 
 PROTOCOL_(BridgedNSError)
 PROTOCOL_(BridgedStoredNSError)

lib/IRGen/GenMeta.cpp

@@ -4370,6 +4370,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
   case KnownProtocolKind::CaseIterable:
   case KnownProtocolKind::Comparable:
   case KnownProtocolKind::SIMDScalar:
+  case KnownProtocolKind::BinaryInteger:
   case KnownProtocolKind::ObjectiveCBridgeable:
   case KnownProtocolKind::DestructorSafeContainer:
   case KnownProtocolKind::SwiftNewtypeWrapper:

lib/Sema/CSDiagnostics.cpp

@@ -2308,14 +2308,68 @@ bool ContextualFailure::diagnoseConversionToBool() const {
     return true;
   }
 
+  // Determine if the boolean negation operator was applied to the anchor. This
+  // upwards traversal of the AST is somewhat fragile, but enables much better
+  // diagnostics if someone attempts to use an optional or integer as a boolean
+  // condition.
+  SourceLoc notOperatorLoc;
+  if (Expr *parent = findParentExpr(getAnchor())) {
+    if (isa<ParenExpr>(parent) && parent->isImplicit()) {
+      if ((parent = findParentExpr(parent))) {
+        auto parentOperatorApplication = dyn_cast<PrefixUnaryExpr>(parent);
+        if (parentOperatorApplication) {
+          auto operatorRefExpr =
+              dyn_cast<DeclRefExpr>(parentOperatorApplication->getFn());
+          if (operatorRefExpr && operatorRefExpr->getDecl()->getBaseName() ==
+                                     getASTContext().Id_NegationOperator) {
+            notOperatorLoc = operatorRefExpr->getLoc();
+          }
+        }
+      }
+    }
+  }
+
   // If we're trying to convert something from optional type to Bool, then a
   // comparison against nil was probably expected.
-  // TODO: It would be nice to handle "!x" --> x == false, but we have no way
-  // to get to the parent expr at present.
   auto fromType = getFromType();
   if (fromType->getOptionalObjectType()) {
     StringRef prefix = "((";
-    StringRef suffix = ") != nil)";
+    StringRef suffix;
+    if (notOperatorLoc.isValid())
+      suffix = ") == nil)";
+    else
+      suffix = ") != nil)";
+
+    // Check if we need the inner parentheses.
+    // Technically we only need them if there's something in 'expr' with
+    // lower precedence than '!=', but the code actually comes out nicer
+    // in most cases with parens on anything non-trivial.
+    if (expr->canAppendPostfixExpression()) {
+      prefix = prefix.drop_back();
+      suffix = suffix.drop_front();
+    }
+    // FIXME: The outer parentheses may be superfluous too.
+
+    emitDiagnostic(expr->getLoc(), diag::optional_used_as_boolean, fromType,
+                   notOperatorLoc.isValid())
+        .fixItInsert(expr->getStartLoc(), prefix)
+        .fixItInsertAfter(expr->getEndLoc(), suffix)
+        .fixItRemove(notOperatorLoc);
+    return true;
+  }
+
+  // If we're trying to convert something from optional type to an integer, then
+  // a comparison against nil was probably expected.
+  auto &cs = getConstraintSystem();
+  if (conformsToKnownProtocol(cs, fromType, KnownProtocolKind::BinaryInteger) &&
+      conformsToKnownProtocol(cs, fromType,
+                              KnownProtocolKind::ExpressibleByIntegerLiteral)) {
+    StringRef prefix = "((";
+    StringRef suffix;
+    if (notOperatorLoc.isValid())
+      suffix = ") == 0)";
+    else
+      suffix = ") != 0)";
 
     // Check if we need the inner parentheses.
     // Technically we only need them if there's something in 'expr' with
@@ -2327,9 +2381,11 @@ bool ContextualFailure::diagnoseConversionToBool() const {
     }
     // FIXME: The outer parentheses may be superfluous too.
 
-    emitDiagnostic(expr->getLoc(), diag::optional_used_as_boolean, fromType)
+    emitDiagnostic(expr->getLoc(), diag::integer_used_as_boolean, fromType,
+                   notOperatorLoc.isValid())
         .fixItInsert(expr->getStartLoc(), prefix)
-        .fixItInsertAfter(expr->getEndLoc(), suffix);
+        .fixItInsertAfter(expr->getEndLoc(), suffix)
+        .fixItRemove(notOperatorLoc);
     return true;
   }
 

test/ClangImporter/private_frameworks_modules.swift

@@ -13,7 +13,7 @@ import PrivateAsParallel_Private
 #error("OLD or NEW must be defined")
 #endif
 
-let _: Bool = PSGlobal // expected-error {{cannot convert value of type 'Int32'}}
-let _: Bool = PSPrivateGlobal // expected-error {{cannot convert value of type 'Int32'}}
-let _: Bool = PPGlobal // expected-error {{cannot convert value of type 'Int32'}}
-let _: Bool = PPPrivateGlobal // expected-error {{cannot convert value of type 'Int32'}}
+let _: Bool = PSGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
+let _: Bool = PSPrivateGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
+let _: Bool = PPGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
+let _: Bool = PPPrivateGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}

test/Constraints/diagnostics.swift

@@ -195,10 +195,10 @@ func r17224804(_ monthNumber : Int) {
 
 // <rdar://problem/17020197> QoI: Operand of postfix '!' should have optional type; type is 'Int?'
 func r17020197(_ x : Int?, y : Int) {
-  if x! {  }  // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  if x! {  }  // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
   // <rdar://problem/12939553> QoI: diagnostic for using an integer in a condition is utterly terrible
-  if y {}    // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  if y {}    // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 }
 
 // <rdar://problem/20714480> QoI: Boolean expr not treated as Bool type when function return type is different
@@ -634,11 +634,11 @@ func r18397777(_ d : r21447318?) {
   if d {  // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '!= nil' instead}} {{6-6=(}} {{7-7= != nil)}}
   }
 
-  if !d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '!= nil' instead}} {{7-7=(}} {{8-8= != nil)}}
+  if !d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '== nil' instead}} {{6-7=}} {{7-7=(}} {{8-8= == nil)}}
 
   }
 
-  if !Optional(c) { // expected-error {{optional type 'Optional<r21447318>' cannot be used as a boolean; test for '!= nil' instead}} {{7-7=(}} {{18-18= != nil)}}
+  if !Optional(c) { // expected-error {{optional type 'Optional<r21447318>' cannot be used as a boolean; test for '== nil' instead}} {{6-7=}} {{7-7=(}} {{18-18= == nil)}}
   }
 }
 

test/Constraints/fixes.swift

@@ -129,11 +129,24 @@ var ciuo: C! = nil
 if co {} // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{4-4=(}} {{6-6= != nil)}}
 if ciuo {} // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{4-4=(}} {{8-8= != nil)}}
 co ? true : false // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{1-1=(}} {{3-3= != nil)}}
-!co ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{4-4= != nil)}}
+!co ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{4-4= == nil)}}
 ciuo ? true : false // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{1-1=(}} {{5-5= != nil)}}
-!ciuo ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{6-6= != nil)}}
-!co // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{4-4= != nil)}}
-!ciuo // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{6-6= != nil)}}
+!ciuo ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{6-6= == nil)}}
+!co // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{4-4= == nil)}}
+!ciuo // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{6-6= == nil)}}
+
+// Used an integer in a conditional expression
+var n1: Int = 1
+var n2: UInt8 = 0
+var n3: Int16 = 2
+
+if n1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}} {{4-4=(}} {{6-6= != 0)}}
+if !n1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '== 0' instead}} {{4-5=}} {{5-5=(}} {{7-7= == 0)}}
+n1 ? true : false // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}} {{1-1=(}} {{3-3= != 0)}}
+!n1 ? false : true // expected-error {{type 'Int' cannot be used as a boolean; test for '== 0' instead}} {{1-2=}} {{2-2=(}} {{4-4= == 0)}}
+!n1 // expected-error {{type 'Int' cannot be used as a boolean; test for '== 0' instead}} {{1-2=}} {{2-2=(}} {{4-4= == 0)}}
+if n2 {} // expected-error {{type 'UInt8' cannot be used as a boolean; test for '!= 0' instead}} {{4-4=(}} {{6-6= != 0)}}
+!n3 // expected-error {{type 'Int16' cannot be used as a boolean; test for '== 0' instead}} {{1-2=}} {{2-2=(}} {{4-4= == 0)}}
 
 // Forgotten ! or ?
 var someInt = co.a // expected-error{{value of optional type 'C?' must be unwrapped to refer to member 'a' of wrapped base type 'C'}}

test/Constraints/if_expr.swift

@@ -28,7 +28,7 @@ useDouble(c)
 useDouble(d)
 
 var z = true ? a : b // expected-error{{result values in '? :' expression have mismatching types 'Int' and 'Double'}}
-var _ = a ? b : b // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+var _ = a ? b : b // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
 
 
@@ -51,9 +51,9 @@ useD1(i) // expected-error{{cannot convert value of type 'B' to expected argumen
 useD2(i) // expected-error{{cannot convert value of type 'B' to expected argument type 'D2'}}
 
 var x = true ? 1 : 0
-var y = 22 ? 1 : 0 // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+var y = 22 ? 1 : 0 // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
-_ = x ? x : x // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+_ = x ? x : x // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 _ = true ? x : 1.2 // expected-error {{result values in '? :' expression have mismatching types 'Int' and 'Double'}}
 
 _ = (x: true) ? true : false // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}

test/Misc/misc_diagnostics.swift

@@ -22,8 +22,8 @@ let total = 15.0
 let count = 7
 let median = total / count // expected-error {{binary operator '/' cannot be applied to operands of type 'Double' and 'Int'}} expected-note {{overloads for '/' exist with these partially matching parameter lists:}}
 
-if (1) {} // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
-if 1 {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+if (1) {} // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
+if 1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
 var a: [String] = [1] // expected-error{{cannot convert value of type 'Int' to expected element type 'String'}}
 var b: Int = [1, 2, 3] // expected-error{{cannot convert value of type '[Int]' to specified type 'Int'}}
@@ -34,7 +34,7 @@ var f2: Float = 3.0
 var dd: Double = f1 - f2 // expected-error{{cannot convert value of type 'Float' to specified type 'Double'}}
 
 func f() -> Bool {
-  return 1 + 1 // expected-error{{cannot convert return expression of type 'Int' to return type 'Bool'}}
+  return 1 + 1 // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 }
 
 // Test that nested diagnostics are properly surfaced.

test/NameBinding/import-resolution-overload.swift

@@ -32,7 +32,7 @@ ambiguousWithVar(true)   // no-warning
 
 var localVar : Bool
 localVar = false
-localVar = 42 // expected-error {{cannot assign value of type 'Int' to type 'Bool'}}
+localVar = 42 // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 localVar(42)  // expected-error {{cannot call value of non-function type 'Bool'}}
 var _ : localVar // should still work
 

test/Parse/switch.swift

@@ -64,7 +64,7 @@ case _ where x % 2 == 0,
   x = 1
 case var y where y % 2 == 0:
   x = y + 1
-case _ where 0: // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+case _ where 0: // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
   x = 0
 default:
   x = 1

test/Sema/pound_assert.swift

@@ -8,6 +8,6 @@
 
 #assert(false, "error message")
 
-#assert(123) // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+#assert(123) // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
-#assert(123, "error message") // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+#assert(123, "error message") // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

test/expr/expressions.swift

@@ -36,7 +36,7 @@ func basictest() {
 
   var x4 : Bool = true
   var x5 : Bool =
-        4 // expected-error {{cannot convert value of type 'Int' to specified type 'Bool'}}
+        4 // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
   //var x6 : Float = 4+5
 
@@ -295,7 +295,7 @@ func fib(_ n: Int) -> Int {
 
 // FIXME: Should warn about integer constants being too large <rdar://problem/14070127>
 var
-   il_a: Bool = 4  // expected-error {{cannot convert value of type 'Int' to specified type 'Bool'}}
+   il_a: Bool = 4  // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 var il_b: Int8
    = 123123
 var il_c: Int8 = 4  // ok
@@ -804,13 +804,13 @@ func testParenExprInTheWay() {
   let x = 42
 
   if x & 4.0 {}  // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
-  // expected-error@-1 {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  // expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
   if (x & 4.0) {}   // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
-  // expected-error@-1 {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  // expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
   if !(x & 4.0) {}  // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
-  // expected-error@-1 {{cannot convert value of type 'Int' to expected argument type 'Bool'}}
+  // expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
-  if x & x {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  if x & x {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 }
 
 // <rdar://problem/21352576> Mixed method/property overload groups can cause a crash during constraint optimization

test/stmt/statements.swift

@@ -59,7 +59,7 @@ func funcdecl5(_ a: Int, y: Int) {
   }
 
   // This diagnostic is terrible - rdar://12939553
-  if x {}   // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  if x {}   // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
 
   if true {
     if (B) {
@@ -583,7 +583,7 @@ func fn(x: Int) {
 }
 
 func bad_if() {
-  if 1 {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
+  if 1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
   if (x: false) {} // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}
   if (x: 1) {} // expected-error {{cannot convert value of type '(x: Int)' to expected condition type 'Bool'}}
   if nil {} // expected-error {{'nil' is not compatible with expected condition type 'Bool'}}