Provide fix-its when implementing a protocol method with bridged types. (#2799) (#2823)

This is the protocol version of 7bfdd4a: if a protocol requirement
has a newly-bridged type (say, 'URL' instead of 'NSURL'), then any
conforming types will need to update their implementations of the
requirement. Reuse the override-checking mechanism to do so when
we're reasonably confident about it.

This slots the checking into the existing protocol diagnostics, which
doesn't result in the best user experience.

    note: protocol requires property 'prop' with type 'Refrigerator?'
      var prop: Refrigerator? { get }
          ^
    note: candidate has non-matching type 'APPRefrigerator?'
      var prop: APPRefrigerator? {
          ^     ~~~~~~~~~~~~~~~~
                Refrigerator?

But we can come back and improve that later; right now this is better
than nothing.

rdar://problem/26237030
(cherry picked from commit 258e2bb)

Author: jrose-apple

lib/Sema/MiscDiagnostics.cpp

@@ -997,6 +997,115 @@ bool swift::diagnoseArgumentLabelError(TypeChecker &TC, const Expr *expr,
   return true;
 }
 
+bool swift::fixItOverrideDeclarationTypes(TypeChecker &TC,
+                                          InFlightDiagnostic &diag,
+                                          ValueDecl *decl,
+                                          const ValueDecl *base) {
+  // For now, just rewrite cases where the base uses a value type and the
+  // override uses a reference type, and the value type is bridged to the
+  // reference type. This is a way to migrate code that makes use of types
+  // that previously were not bridged to value types.
+  auto checkType = [&](Type overrideTy, Type baseTy,
+                       SourceRange typeRange) -> bool {
+    if (typeRange.isInvalid())
+      return false;
+    if (!baseTy) {
+      decl->dump();
+      base->dump();
+    }
+
+    auto normalizeType = [](Type ty) -> Type {
+      ty = ty->getInOutObjectType();
+      if (Type unwrappedTy = ty->getAnyOptionalObjectType())
+        ty = unwrappedTy;
+      return ty;
+    };
+
+    // Is the base type bridged?
+    Type normalizedBaseTy = normalizeType(baseTy);
+    const DeclContext *DC = decl->getDeclContext();
+    Optional<Type> maybeBridged =
+        TC.Context.getBridgedToObjC(DC, normalizedBaseTy, &TC);
+
+    // ...and just knowing that it's bridged isn't good enough if we don't
+    // know what it's bridged /to/. Also, don't do this check for trivial
+    // bridging---that doesn't count.
+    Type bridged = maybeBridged.getValueOr(Type());
+    if (!bridged || bridged->isEqual(normalizedBaseTy))
+      return false;
+
+    // ...and is it bridged to the overridden type?
+    Type normalizedOverrideTy = normalizeType(overrideTy);
+    if (!bridged->isEqual(normalizedOverrideTy)) {
+      // If both are nominal types, check again, ignoring generic arguments.
+      auto *overrideNominal = normalizedOverrideTy->getAnyNominal();
+      if (!overrideNominal || bridged->getAnyNominal() != overrideNominal) {
+        return false;
+      }
+    }
+
+    Type newOverrideTy = baseTy;
+
+    // Preserve optionality if we're dealing with a simple type.
+    OptionalTypeKind OTK;
+    if (Type unwrappedTy = newOverrideTy->getAnyOptionalObjectType())
+      newOverrideTy = unwrappedTy;
+    if (overrideTy->getAnyOptionalObjectType(OTK))
+      newOverrideTy = OptionalType::get(OTK, newOverrideTy);
+
+    SmallString<32> baseTypeBuf;
+    llvm::raw_svector_ostream baseTypeStr(baseTypeBuf);
+    PrintOptions options;
+    options.SynthesizeSugarOnTypes = true;
+
+    newOverrideTy->print(baseTypeStr, options);
+    diag.fixItReplace(typeRange, baseTypeStr.str());
+    return true;
+  };
+
+  if (auto *var = dyn_cast<VarDecl>(decl)) {
+    SourceRange typeRange = var->getTypeSourceRangeForDiagnostics();
+    return checkType(var->getType(), base->getType(), typeRange);
+  }
+
+  if (auto *fn = dyn_cast<AbstractFunctionDecl>(decl)) {
+    auto *baseFn = cast<AbstractFunctionDecl>(base);
+    bool fixedAny = false;
+    if (fn->getParameterLists().back()->size() ==
+        baseFn->getParameterLists().back()->size()) {
+      for_each(*fn->getParameterLists().back(),
+               *baseFn->getParameterLists().back(),
+               [&](ParamDecl *param, const ParamDecl *baseParam) {
+        fixedAny |= fixItOverrideDeclarationTypes(TC, diag, param, baseParam);
+      });
+    }
+    if (auto *method = dyn_cast<FuncDecl>(decl)) {
+      auto *baseMethod = cast<FuncDecl>(base);
+      fixedAny |= checkType(method->getBodyResultType(),
+                            baseMethod->getResultType(),
+                            method->getBodyResultTypeLoc().getSourceRange());
+    }
+    return fixedAny;
+  }
+
+  if (auto *subscript = dyn_cast<SubscriptDecl>(decl)) {
+    auto *baseSubscript = cast<SubscriptDecl>(base);
+    bool fixedAny = false;
+    for_each(*subscript->getIndices(),
+             *baseSubscript->getIndices(),
+             [&](ParamDecl *param, const ParamDecl *baseParam) {
+      fixedAny |= fixItOverrideDeclarationTypes(TC, diag, param, baseParam);
+    });
+    fixedAny |= checkType(subscript->getElementType(),
+                          baseSubscript->getElementType(),
+                          subscript->getElementTypeLoc().getSourceRange());
+    return fixedAny;
+  }
+
+  llvm_unreachable("unknown overridable member");
+}
+
+
 
 //===----------------------------------------------------------------------===//
 // Diagnose availability.

lib/Sema/MiscDiagnostics.h

@@ -68,6 +68,16 @@ void fixItAvailableAttrRename(TypeChecker &TC,
                               SourceRange referenceRange,
                               const AvailableAttr *attr,
                               const CallExpr *CE);
+
+/// Attempt to fix the type of \p decl so that it's a valid override for
+/// \p base...but only if we're highly confident that we know what the user
+/// should have written.
+///
+/// \returns true iff any fix-its were attached to \p diag.
+bool fixItOverrideDeclarationTypes(TypeChecker &TC,
+                                   InFlightDiagnostic &diag,
+                                   ValueDecl *decl,
+                                   const ValueDecl *base);
 } // namespace swift
 
 #endif // SWIFT_SEMA_MISC_DIAGNOSTICS_H

lib/Sema/TypeCheckDecl.cpp

@@ -4810,115 +4810,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       type = fnType->withExtInfo(extInfo);
   }
 
-  /// Attempt to fix the type of \p decl so that it's a valid override for
-  /// \p base...but only if we're highly confident that we know what the user
-  /// should have written.
-  ///
-  /// \returns true iff any fix-its were attached to \p diag.
-  static bool fixOverrideDeclarationTypes(InFlightDiagnostic &diag,
-                                          TypeChecker &TC,
-                                          ValueDecl *decl,
-                                          const ValueDecl *base) {
-    // For now, just rewrite cases where the base uses a value type and the
-    // override uses a reference type, and the value type is bridged to the
-    // reference type. This is a way to migrate code that makes use of types
-    // that previously were not bridged to value types.
-    auto checkType = [&](Type overrideTy, Type baseTy,
-                         SourceRange typeRange) -> bool {
-      if (typeRange.isInvalid())
-        return false;
-
-      auto normalizeType = [](Type ty) -> Type {
-        ty = ty->getInOutObjectType();
-        if (Type unwrappedTy = ty->getAnyOptionalObjectType())
-          ty = unwrappedTy;
-        return ty;
-      };
-
-      // Is the base type bridged?
-      Type normalizedBaseTy = normalizeType(baseTy);
-      const DeclContext *DC = decl->getDeclContext();
-      Optional<Type> maybeBridged =
-          TC.Context.getBridgedToObjC(DC, normalizedBaseTy, &TC);
-
-      // ...and just knowing that it's bridged isn't good enough if we don't
-      // know what it's bridged /to/. Also, don't do this check for trivial
-      // bridging---that doesn't count.
-      Type bridged = maybeBridged.getValueOr(Type());
-      if (!bridged || bridged->isEqual(normalizedBaseTy))
-        return false;
-
-      // ...and is it bridged to the overridden type?
-      Type normalizedOverrideTy = normalizeType(overrideTy);
-      if (!bridged->isEqual(normalizedOverrideTy)) {
-        // If both are nominal types, check again, ignoring generic arguments.
-        auto *overrideNominal = normalizedOverrideTy->getAnyNominal();
-        if (!overrideNominal || bridged->getAnyNominal() != overrideNominal) {
-          return false;
-        }
-      }
-
-      Type newOverrideTy = baseTy;
-
-      // Preserve optionality if we're dealing with a simple type.
-      OptionalTypeKind OTK;
-      if (Type unwrappedTy = newOverrideTy->getAnyOptionalObjectType())
-        newOverrideTy = unwrappedTy;
-      if (overrideTy->getAnyOptionalObjectType(OTK))
-        newOverrideTy = OptionalType::get(OTK, newOverrideTy);
-
-      SmallString<32> baseTypeBuf;
-      llvm::raw_svector_ostream baseTypeStr(baseTypeBuf);
-      PrintOptions options;
-      options.SynthesizeSugarOnTypes = true;
-
-      newOverrideTy->print(baseTypeStr, options);
-      diag.fixItReplace(typeRange, baseTypeStr.str());
-      return true;
-    };
-
-    if (auto *var = dyn_cast<VarDecl>(decl)) {
-      SourceRange typeRange = var->getTypeSourceRangeForDiagnostics();
-      return checkType(var->getType(), base->getType(), typeRange);
-    }
-
-    if (auto *fn = dyn_cast<AbstractFunctionDecl>(decl)) {
-      auto *baseFn = cast<AbstractFunctionDecl>(base);
-      bool fixedAny = false;
-      if (fn->getParameterLists().back()->size() ==
-          baseFn->getParameterLists().back()->size()) {
-        for_each(*fn->getParameterLists().back(),
-                 *baseFn->getParameterLists().back(),
-                 [&](ParamDecl *param, const ParamDecl *baseParam) {
-          fixedAny |= fixOverrideDeclarationTypes(diag, TC, param, baseParam);
-        });
-      }
-      if (auto *method = dyn_cast<FuncDecl>(decl)) {
-        auto *baseMethod = cast<FuncDecl>(base);
-        fixedAny |= checkType(method->getBodyResultType(),
-                              baseMethod->getBodyResultType(),
-                              method->getBodyResultTypeLoc().getSourceRange());
-      }
-      return fixedAny;
-    }
-
-    if (auto *subscript = dyn_cast<SubscriptDecl>(decl)) {
-      auto *baseSubscript = cast<SubscriptDecl>(base);
-      bool fixedAny = false;
-      for_each(*subscript->getIndices(),
-               *baseSubscript->getIndices(),
-               [&](ParamDecl *param, const ParamDecl *baseParam) {
-        fixedAny |= fixOverrideDeclarationTypes(diag, TC, param, baseParam);
-      });
-      fixedAny |= checkType(subscript->getElementType(),
-                            baseSubscript->getElementType(),
-                            subscript->getElementTypeLoc().getSourceRange());
-      return fixedAny;
-    }
-
-    llvm_unreachable("unknown overridable member");
-  }
-
   /// If the difference between the types of \p decl and \p base is something
   /// we feel confident about fixing (even partially), emit a note with fix-its
   /// attached. Otherwise, no note will be emitted.
@@ -4953,7 +4844,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
                                        base->getDescriptiveKind(), baseTy));
       }
 
-      if (fixOverrideDeclarationTypes(*activeDiag, TC, decl, base))
+      if (fixItOverrideDeclarationTypes(TC, *activeDiag, decl, base))
         return true;
     }
 

lib/Sema/TypeCheckProtocol.cpp

@@ -1733,11 +1733,14 @@ diagnoseMatch(TypeChecker &tc, Module *module,
     // about them.
     break;
 
-  case MatchKind::TypeConflict:
-    tc.diagnose(match.Witness, diag::protocol_witness_type_conflict,
-                getTypeForDisplay(tc, module, match.Witness),
-                withAssocTypes);
+  case MatchKind::TypeConflict: {
+    auto diag = tc.diagnose(match.Witness, diag::protocol_witness_type_conflict,
+                            getTypeForDisplay(tc, module, match.Witness),
+                            withAssocTypes);
+    if (!isa<TypeDecl>(req))
+      fixItOverrideDeclarationTypes(tc, diag, match.Witness, req);
     break;
+  }
 
   case MatchKind::ThrowsConflict:
     tc.diagnose(match.Witness, diag::protocol_witness_throws_conflict);