Merge pull request #21124 from xedin/rdar-45218255-5.0

[5.0][CSDiagnostics] Diagnose invalid optional unwrap via fixes

Author: AnnaZaks

lib/Sema/CSDiagnostics.cpp

@@ -1222,3 +1222,17 @@ bool AutoClosureForwardingFailure::diagnoseAsError() {
       .fixItInsertAfter(argExpr->getEndLoc(), "()");
   return true;
 }
+
+bool NonOptionalUnwrapFailure::diagnoseAsError() {
+  auto *anchor = getAnchor();
+
+  auto diagnostic = diag::invalid_optional_chain;
+  if (isa<ForceValueExpr>(anchor))
+    diagnostic = diag::invalid_force_unwrap;
+
+  emitDiagnostic(anchor->getLoc(), diagnostic, BaseType)
+      .highlight(anchor->getSourceRange())
+      .fixItRemove(anchor->getEndLoc());
+
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -607,6 +607,29 @@ class AutoClosureForwardingFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose situations when there was an attempt to unwrap entity
+/// of non-optional type e.g.
+///
+/// ```swift
+/// let i: Int = 0
+/// _ = i!
+///
+/// struct A { func foo() {} }
+/// func foo(_ a: A) {
+///   a?.foo()
+/// }
+/// ```
+class NonOptionalUnwrapFailure final : public FailureDiagnostic {
+  Type BaseType;
+
+public:
+  NonOptionalUnwrapFailure(Expr *root, ConstraintSystem &cs, Type baseType,
+                           ConstraintLocator *locator)
+      : FailureDiagnostic(root, cs, locator), BaseType(baseType) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -213,3 +213,14 @@ AutoClosureForwarding *AutoClosureForwarding::create(ConstraintSystem &cs,
                                                      ConstraintLocator *locator) {
   return new (cs.getAllocator()) AutoClosureForwarding(cs, locator);
 }
+
+bool RemoveUnwrap::diagnose(Expr *root, bool asNote) const {
+  auto failure = NonOptionalUnwrapFailure(root, getConstraintSystem(), BaseType,
+                                          getLocator());
+  return failure.diagnose(asNote);
+}
+
+RemoveUnwrap *RemoveUnwrap::create(ConstraintSystem &cs, Type baseType,
+                                   ConstraintLocator *locator) {
+  return new (cs.getAllocator()) RemoveUnwrap(cs, baseType, locator);
+}

lib/Sema/CSFix.h

@@ -89,6 +89,9 @@ enum class FixKind : uint8_t {
   /// @autoclosure conversions are unsupported starting from
   /// Swift version 5.
   AutoClosureForwarding,
+
+  /// Remove `!` or `?` because base is not an optional type.
+  RemoveUnwrap,
 };
 
 class ConstraintFix {
@@ -408,6 +411,23 @@ class AutoClosureForwarding final : public ConstraintFix {
                                        ConstraintLocator *locator);
 };
 
+class RemoveUnwrap final : public ConstraintFix {
+  Type BaseType;
+
+public:
+  RemoveUnwrap(ConstraintSystem &cs, Type baseType, ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::RemoveUnwrap, locator), BaseType(baseType) {}
+
+  std::string getName() const override {
+    return "remove unwrap operator `!` or `?`";
+  }
+
+  bool diagnose(Expr *root, bool asNote = false) const override;
+
+  static RemoveUnwrap *create(ConstraintSystem &cs, Type baseType,
+                              ConstraintLocator *locator);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSSimplify.cpp

@@ -2979,10 +2979,27 @@ ConstraintSystem::simplifyOptionalObjectConstraint(
     return SolutionKind::Unsolved;
   }
 
-  // If the base type is not optional, the constraint fails.
+
   Type objectTy = optTy->getOptionalObjectType();
-  if (!objectTy)
-    return SolutionKind::Error;
+  // If the base type is not optional, let's attempt a fix (if possible)
+  // and assume that `!` is just not there.
+  if (!objectTy) {
+    // Let's see if we can apply a specific fix here.
+    if (shouldAttemptFixes()) {
+      auto *fix =
+          RemoveUnwrap::create(*this, optTy, getConstraintLocator(locator));
+
+      if (recordFix(fix))
+        return SolutionKind::Error;
+
+      // If the fix was successful let's record
+      // "fixed" object type and continue.
+      objectTy = optTy;
+    } else {
+      // If fixes are not allowed, no choice but to fail.
+      return SolutionKind::Error;
+    }
+  }
 
   // The object type is an lvalue if the optional was.
   if (optLValueTy->is<LValueType>())
@@ -5427,6 +5444,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::RelabelArguments:
   case FixKind::AddConformance:
   case FixKind::AutoClosureForwarding:
+  case FixKind::RemoveUnwrap:
     llvm_unreachable("handled elsewhere");
   }
 

test/Constraints/diagnostics.swift

@@ -1183,6 +1183,14 @@ func rdar17170728() {
 
   let _ = [i, j, k].reduce(0 as Int?) {
     $0 && $1 ? $0! + $1! : ($0 ? $0! : ($1 ? $1! : nil))
+    // expected-error@-1 {{cannot force unwrap value of non-optional type 'Bool'}} {{18-19=}}
+    // expected-error@-2 {{cannot force unwrap value of non-optional type 'Bool'}} {{24-25=}}
+    // expected-error@-3 {{cannot force unwrap value of non-optional type 'Bool'}} {{36-37=}}
+    // expected-error@-4 {{cannot force unwrap value of non-optional type 'Bool'}} {{48-49=}}
+  }
+
+  let _ = [i, j, k].reduce(0 as Int?) {
+    $0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
     // expected-error@-1 {{ambiguous use of operator '+'}}
   }
 }

test/Constraints/dynamic_lookup.swift

@@ -210,7 +210,6 @@ type(of: obj).foo!(obj)(5) // expected-error{{instance member 'foo' cannot be us
 
 // Checked casts to AnyObject
 var p: P = Y()
-// expected-warning @+1 {{forced cast from 'P' to 'AnyObject' always succeeds; did you mean to use 'as'?}}
 var obj3 : AnyObject = (p as! AnyObject)! // expected-error{{cannot force unwrap value of non-optional type 'AnyObject'}} {{41-42=}}
 
 // Implicit force of an implicitly unwrapped optional

test/Constraints/optional.swift

@@ -292,3 +292,13 @@ func se0213() {
   _ = Q("who")!.foo // Ok
   _ = Q?("how") // Ok
 }
+
+func rdar45218255(_ i: Int) {
+  struct S<T> {
+    init(_:[T]) {}
+  }
+
+  _ = i!           // expected-error {{cannot force unwrap value of non-optional type 'Int'}} {{8-9=}}
+  _ = [i!]         // expected-error {{cannot force unwrap value of non-optional type 'Int'}} {{9-10=}}
+  _ = S<Int>([i!]) // expected-error {{cannot force unwrap value of non-optional type 'Int'}} {{16-17=}}
+}

test/Parse/operators.swift

@@ -116,8 +116,11 @@ infix operator !!
 func !!(x: Man, y: Man) {}
 let foo = Man()
 let bar = TheDevil()
-foo!!foo // expected-error{{cannot force unwrap value of non-optional type 'Man'}} {{4-5=}} expected-error{{consecutive statements}} {{6-6=;}}
-// expected-warning @-1 {{expression of type 'Man' is unused}}
+foo!!foo
+// expected-error@-1 {{cannot force unwrap value of non-optional type 'Man'}} {{4-5=}}
+// expected-error@-2 {{cannot force unwrap value of non-optional type 'Man'}} {{5-6=}}
+// expected-error@-3 {{consecutive statements}} {{6-6=;}}
+// expected-warning@-4 {{expression of type 'Man' is unused}}
 
 foo??bar // expected-error{{broken standard library}} expected-error{{consecutive statements}} {{6-6=;}}
 // expected-warning @-1 {{expression of type 'TheDevil' is unused}}

test/decl/init/failable.swift

@@ -60,7 +60,7 @@ class Sub : Super {
   }
 
   convenience init(forceNonfail: Int) {
-    self.init(nonfail: forceNonfail)! // expected-error{{cannot force unwrap value of non-optional type '()'}} {{37-38=}}
+    self.init(nonfail: forceNonfail)! // expected-error{{cannot force unwrap value of non-optional type 'Sub'}} {{37-38=}}
   }
 
   init(nonfail2: Int) { // okay, traps on nil