Merge pull request #22802 from xcadaverx/xcadaverx/SR-9851_Incorrect_fix_it_logical_not_optional_bool

[Sema] SR-9851 - Add parens when needed for nil coalescing fixits

Author: xedin

lib/Sema/CSApply.cpp

@@ -7626,10 +7626,14 @@ bool swift::exprNeedsParensOutsideFollowingOperator(
   Expr *parent;
   unsigned index;
   std::tie(parent, index) = getPrecedenceParentAndIndex(expr, rootExpr);
-  if (!parent || isa<TupleExpr>(parent) || isa<ParenExpr>(parent)) {
+  if (!parent || isa<TupleExpr>(parent)) {
     return false;
   }
 
+  if (auto parenExp = dyn_cast<ParenExpr>(parent))
+    if (!parenExp->isImplicit())
+      return false;
+
   if (parent->isInfixOperator()) {
     auto parentPG = TC.lookupPrecedenceGroupForInfixOperator(DC, parent);
     if (!parentPG) return true;

lib/Sema/CSDiagnostics.cpp

@@ -546,15 +546,15 @@ bool MemberAccessOnOptionalBaseFailure::diagnoseAsError() {
 }
 
 // Suggest a default value via ?? <default value>
-static void offerDefaultValueUnwrapFixit(TypeChecker &TC, DeclContext *DC, Expr *expr) {
+static void offerDefaultValueUnwrapFixit(TypeChecker &TC, DeclContext *DC, Expr *expr, Expr *rootExpr) {
   auto diag =
   TC.diagnose(expr->getLoc(), diag::unwrap_with_default_value);
 
   // Figure out what we need to parenthesize.
   bool needsParensInside =
   exprNeedsParensBeforeAddingNilCoalescing(TC, DC, expr);
   bool needsParensOutside =
-  exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, expr);
+  exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, rootExpr);
 
   llvm::SmallString<2> insertBefore;
   llvm::SmallString<32> insertAfter;
@@ -618,7 +618,7 @@ class VarDeclMultipleReferencesChecker : public ASTWalker {
   int referencesCount() { return count; }
 };
 
-static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type baseType,
+static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Expr *rootExpr, Type baseType,
                            Type unwrappedType) {
 
   assert(!baseType->hasTypeVariable() &&
@@ -673,14 +673,14 @@ static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Type baseType,
         }
         diag.flush();
 
-        offerDefaultValueUnwrapFixit(CS.TC, varDecl->getDeclContext(),
-                                     initializer);
+        offerDefaultValueUnwrapFixit(CS.TC, varDecl->getDeclContext(), 
+                                     initializer, rootExpr);
         offerForceUnwrapFixit(CS, initializer);
       }
     }
   }
 
-  offerDefaultValueUnwrapFixit(CS.TC, CS.DC, expr);
+  offerDefaultValueUnwrapFixit(CS.TC, CS.DC, expr, rootExpr);
   offerForceUnwrapFixit(CS, expr);
   return true;
 }
@@ -690,6 +690,7 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
     return false;
 
   auto *anchor = getAnchor();
+  auto *rootExpr = getParentExpr();
 
   if (auto assignExpr = dyn_cast<AssignExpr>(anchor))
     anchor = assignExpr->getSrc();
@@ -699,7 +700,7 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
 
   auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
   if (!tryExpr) {
-    return diagnoseUnwrap(getConstraintSystem(), unwrapped, getBaseType(),
+    return diagnoseUnwrap(getConstraintSystem(), unwrapped, rootExpr, getBaseType(),
                           getUnwrappedType());
   }
 

test/Constraints/fixes.swift

@@ -287,3 +287,11 @@ let _: Int = thing?.e?.a() // expected-error {{value of optional type 'Int?' mus
 let _: Int? = thing?.e?.b // expected-error {{value of optional type 'Int??' must be unwrapped to a value of type 'Int?'}}
 // expected-note@-1{{coalesce}}
 // expected-note@-2{{force-unwrap}}
+
+// SR-9851 - https://bugs.swift.org/browse/SR-9851
+func coalesceWithParensRootExprFix() {
+  let optionalBool: Bool? = false
+  if !optionalBool { }  // expected-error{{value of optional type 'Bool?' must be unwrapped to a value of type 'Bool'}}
+  // expected-note@-1{{coalesce using '??' to provide a default when the optional value contains 'nil'}}{{7-7=(}}{{19-19= ?? <#default value#>)}}
+  // expected-note@-2{{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+}