General bike shedding

Author: CodaFi

include/swift/AST/DiagnosticsSema.def

@@ -2405,16 +2405,21 @@ WARNING(optional_pattern_match_promotion,none,
         (Type, Type))
 WARNING(optional_to_any_coercion,none,
         "expression implicitly coerced from %0 to Any", (Type))
-WARNING(optional_in_string_interpolation_segment,none,
-        "string interpolation of optional can have unintended effects", ())
 NOTE(default_optional_to_any,none,
      "provide a default value to avoid this warning", ())
 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", ())
-NOTE(silence_optional_in_interpolation_segment,none,
-      "explicitly use '.debugDescription' 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_cast,none,
+     "use 'as' to explicitly cast to %0 to silence this warning",
+     (Type))
+NOTE(silence_optional_in_interpolation_segment_call,none,
+     "use '.debugDescription' to silence this warning", ())
 
 ERROR(invalid_noescape_use,none,
       "non-escaping %select{value|parameter}1 %0 may only be called",

lib/Sema/MiscDiagnostics.cpp

@@ -3672,12 +3672,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;
 
@@ -3711,37 +3711,45 @@ static void diagnoseOptionalToAnyCoercion(TypeChecker &TC, const Expr *E,
         // Warn about interpolated segments that contain optionals.
         for (auto &segment : literal->getSegments()) {
           // Allow explicit casts.
-          if (isa<ExplicitCastExpr>(segment)) {
-            continue;
+          if (auto paren = dyn_cast<ParenExpr>(segment)) {
+            if (isa<ExplicitCastExpr>(paren->getSubExpr())) {
+              continue;
+            }
           }
 
-          auto segmentTy = segment->getType();
-          if (segmentTy && !segmentTy->getOptionalObjectType().isNull()) {
-            TC.diagnose(segment->getStartLoc(),
-                        diag::optional_in_string_interpolation_segment)
-                .highlight(segment->getSourceRange());
-
-            TC.diagnose(segment->getLoc(),
-                        diag::silence_optional_in_interpolation_segment)
-              .highlight(segment->getSourceRange())
-              .fixItInsertAfter(segment->getEndLoc(), ".debugDescription");
-            TC.diagnose(segment->getLoc(), diag::default_optional_to_any)
-              .highlight(segment->getSourceRange())
-            	.fixItInsertAfter(segment->getEndLoc(), " ?? <#default value#>");
-            TC.diagnose(segment->getLoc(), diag::force_optional_to_any)
-              .highlight(segment->getSourceRange())
-              .fixItInsertAfter(segment->getEndLoc(), "!");
+          // Bail out if we don't have an optional.
+          auto objectTy = segment->getType()->getOptionalObjectType();
+          if (!objectTy) {
+            continue;
           }
+          auto segmentTy = OptionalType::get(objectTy);
+
+          TC.diagnose(segment->getStartLoc(),
+                      diag::optional_in_string_interpolation_segment)
+              .highlight(segment->getSourceRange());
+
+          TC.diagnose(segment->getLoc(),
+                      diag::silence_optional_in_interpolation_segment_call)
+            .highlight(segment->getSourceRange())
+            .fixItInsert(segment->getEndLoc(), ".debugDescription");
+
+          auto opts = PrintOptions::printForDiagnostics();
+          TC.diagnose(segment->getLoc(),
+                      diag::silence_optional_in_interpolation_segment_cast,
+                      segmentTy)
+            .highlight(segment->getSourceRange())
+            .fixItInsert(segment->getEndLoc(),
+                         " as " + segmentTy->getString(opts));
         }
       }
       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);
 }
 
@@ -3757,7 +3765,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 renamed to 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 '.debugDescription' to silence this warning}} {{52-52=.debugDescription}}
+  // expected-note@-3 {{use 'as' to explicitly cast to 'Int?' to silence this warning}} {{52-52= as Int?}}
+  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 '.debugDescription' to silence this warning}} {{67-67=.debugDescription}}
+  // expected-note@-3 {{use 'as' to explicitly cast to 'Int?' to silence this warning}} {{67-67= as Int?}}
+
+  print("Always some, Always some, Always some: \(o as Int?)") // No warning
+  print("Always some, Always some, Always some: \(o.debugDescription)") // No warning.
+}
+