[3.0] Warn for Optionals in String Interpolation Segments

include/swift/AST/DiagnosticsSema.def

@@ -2403,6 +2403,12 @@ NOTE(force_optional_to_any,none,
      "force-unwrap the value to avoid this warning", ())
 NOTE(silence_optional_to_any,none,
      "explicitly cast to Any with 'as Any' to silence this warning", ())
+WARNING(optional_in_string_interpolation_segment,none,
+        "string interpolation produces a debug description for an optional "
+        "value; did you mean to make this explicit?",
+        ())
+NOTE(silence_optional_in_interpolation_segment_call,none,
+     "use 'String(describing:)' to silence this warning", ())
 
 ERROR(invalid_noescape_use,none,
       "non-escaping %select{value|parameter}1 %0 may only be called",

lib/Sema/MiscDiagnostics.cpp

@@ -3667,12 +3667,12 @@ checkImplicitPromotionsInCondition(const StmtConditionElement &cond,
   }
 }
 
-static void diagnoseOptionalToAnyCoercion(TypeChecker &TC, const Expr *E,
-                                          const DeclContext *DC) {
+static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E,
+                                               const DeclContext *DC) {
   if (!E || isa<ErrorExpr>(E) || !E->getType())
     return;
 
-  class OptionalToAnyCoercionWalker : public ASTWalker {
+  class UnintendedOptionalBehaviorWalker : public ASTWalker {
     TypeChecker &TC;
     SmallPtrSet<Expr *, 4> ErasureCoercedToAny;
 
@@ -3702,16 +3702,47 @@ static void diagnoseOptionalToAnyCoercion(TypeChecker &TC, const Expr *E,
               .highlight(subExpr->getSourceRange())
               .fixItInsertAfter(subExpr->getEndLoc(), " as Any");
         }
-      }
+      } else if (auto *literal = dyn_cast<InterpolatedStringLiteralExpr>(E)) {
+        // Warn about interpolated segments that contain optionals.
+        for (auto &segment : literal->getSegments()) {
+          // Allow explicit casts.
+          if (auto paren = dyn_cast<ParenExpr>(segment)) {
+            if (isa<ExplicitCastExpr>(paren->getSubExpr())) {
+              continue;
+            }
+          }
 
+          // Bail out if we don't have an optional.
+          if (!segment->getType()->getOptionalObjectType()) {
+            continue;
+          }
+
+          TC.diagnose(segment->getStartLoc(),
+                      diag::optional_in_string_interpolation_segment)
+              .highlight(segment->getSourceRange());
+
+          // Suggest 'String(describing: <expr>)'.
+          auto segmentStart = segment->getStartLoc().getAdvancedLoc(1);
+          TC.diagnose(segment->getLoc(),
+                      diag::silence_optional_in_interpolation_segment_call)
+            .highlight(segment->getSourceRange())
+            .fixItInsert(segmentStart, "String(describing: ")
+            .fixItInsert(segment->getEndLoc(), ")");
+
+          // Suggest inserting a default value. 
+          TC.diagnose(segment->getLoc(), diag::default_optional_to_any)
+            .highlight(segment->getSourceRange())
+            .fixItInsert(segment->getEndLoc(), " ?? <#default value#>");
+        }
+      }
       return { true, E };
     }
 
   public:
-    OptionalToAnyCoercionWalker(TypeChecker &tc) : TC(tc) { }
+    UnintendedOptionalBehaviorWalker(TypeChecker &tc) : TC(tc) { }
   };
 
-  OptionalToAnyCoercionWalker Walker(TC);
+  UnintendedOptionalBehaviorWalker Walker(TC);
   const_cast<Expr *>(E)->walk(Walker);
 }
 
@@ -3727,7 +3758,7 @@ void swift::performSyntacticExprDiagnostics(TypeChecker &TC, const Expr *E,
   diagSyntacticUseRestrictions(TC, E, DC, isExprStmt);
   diagRecursivePropertyAccess(TC, E, DC);
   diagnoseImplicitSelfUseInClosure(TC, E, DC);
-  diagnoseOptionalToAnyCoercion(TC, E, DC);
+  diagnoseUnintendedOptionalBehavior(TC, E, DC);
   if (!TC.getLangOpts().DisableAvailabilityChecking)
     diagAvailability(TC, E, const_cast<DeclContext*>(DC));
   if (TC.Context.LangOpts.EnableObjCInterop)

test/Sema/diag_optional_to_any.swift → test/Sema/diag_unintended_optional_behavior.swift

@@ -58,3 +58,18 @@ func warnOptionalToAnyCoercion(value x: Int?) -> Any {
   // expected-note@-2 {{force-unwrap the value to avoid this warning}}
   // expected-note@-3 {{explicitly cast to Any with 'as Any' to silence this warning}}
 }
+
+func warnOptionalInStringInterpolationSegment(_ o : Int?) {
+  print("Always some, Always some, Always some: \(o)")
+  // expected-warning@-1 {{string interpolation produces a debug description for an optional value; did you mean to make this explicit?}}
+  // expected-note@-2 {{use 'String(describing:)' to silence this warning}} {{51-51=String(describing: }} {{52-52=)}} 
+  // expected-note@-3 {{provide a default value to avoid this warning}} {{52-52= ?? <#default value#>}}
+  print("Always some, Always some, Always some: \(o.map { $0 + 1 })")
+  // expected-warning@-1 {{string interpolation produces a debug description for an optional value; did you mean to make this explicit?}}
+  // expected-note@-2 {{use 'String(describing:)' to silence this warning}} {{51-51=String(describing: }} {{67-67=)}} 
+  // expected-note@-3 {{provide a default value to avoid this warning}} {{67-67= ?? <#default value#>}}
+
+  print("Always some, Always some, Always some: \(o as Int?)") // No warning
+  print("Always some, Always some, Always some: \(o.debugDescription)") // No warning.
+}
+