Provide fix-its when overriding a bridged type with the old type.

This is support for SE-0069: Mutability and Foundation Value Types.
In cases where someone has overridden a method that takes, e.g.
'NSURL', the override will no longer be valid, because the system
class will now use the value type 'URL' instead. If an override's
full name matches exactly, the compiler will offer fix-its for any
uses of reference types where value types are now preferred.
(This must be a direct match; subclasses, including the mutable
variants of many Foundation types, will need to be updated by hand.)

One wrinkle here is the use of generics. In Swift 2, Objective-C
generics weren't imported at all, so it's unlikely that the overriding
method will have the correct generic arguments. Simple migration
might insert the "bound" type, but it can't know what specific type
might be more appropriate. Therefore, the logic to add the fix-it
ignores generic arguments, assuming the parent's type is correct.

rdar://problem/26183575

Author: jrose-apple

include/swift/AST/Decl.h

@@ -4189,7 +4189,8 @@ class VarDecl : public AbstractStorageDecl {
     return VarDeclBits.IsUserAccessible;
   }
 
-  /// \brief Retrieve the source range of the variable type.
+  /// Retrieve the source range of the variable type, or an invalid range if the
+  /// variable's type is not explicitly written in the source.
   ///
   /// Only for use in diagnostics.  It is not always possible to always
   /// precisely point to the variable type because of type aliases.

include/swift/AST/DiagnosticsSema.def

@@ -1460,6 +1460,12 @@ NOTE(subscript_override_here,none,
      "attempt to override subscript here", ())
 NOTE(convenience_init_override_here,none,
      "attempt to override convenience initializer here", ())
+NOTE(override_type_mismatch_with_fixits,none,
+     "type does not match superclass %0 with type %1",
+     (DescriptiveDeclKind, Type))
+NOTE(override_type_mismatch_with_fixits_init,none,
+     "type does not match superclass initializer with %select{no arguments|argument %1|arguments %1}0",
+     (unsigned, Type))
 ERROR(override_nonclass_decl,none,
       "'override' can only be specified on class members", ())
 ERROR(override_property_type_mismatch,none,

lib/AST/ASTVerifier.cpp

@@ -2769,14 +2769,6 @@ struct ASTNodeBase {};
       }
       checkSourceRanges(D->getSourceRange(), Parent,
                         [&]{ D->print(Out); });
-      if (auto *VD = dyn_cast<VarDecl>(D)) {
-        if (!VD->getTypeSourceRangeForDiagnostics().isValid()) {
-          Out << "invalid type source range for variable decl: ";
-          D->print(Out);
-          Out << "\n";
-          abort();
-        }
-      }
     }
 
     /// \brief Verify that the given source ranges is contained within the

lib/AST/Decl.cpp

@@ -3333,13 +3333,14 @@ SourceRange VarDecl::getTypeSourceRangeForDiagnostics() const {
 
   Pattern *Pat = getParentPattern();
   if (!Pat || Pat->isImplicit())
-    return getSourceRange();
+    return SourceRange();
 
   if (auto *VP = dyn_cast<VarPattern>(Pat))
     Pat = VP->getSubPattern();
   if (auto *TP = dyn_cast<TypedPattern>(Pat))
     return TP->getTypeLoc().getTypeRepr()->getSourceRange();
-  return getSourceRange();
+
+  return SourceRange();
 }
 
 static bool isVarInPattern(const VarDecl *VD, Pattern *P) {

lib/Sema/TypeCheckDecl.cpp

@@ -4759,6 +4759,158 @@ 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.
+  ///
+  /// \returns true iff a diagnostic was emitted.
+  static bool noteFixableMismatchedTypes(TypeChecker &TC, ValueDecl *decl,
+                                         const ValueDecl *base) {
+    DiagnosticTransaction tentativeDiags(TC.Diags);
+
+    {
+      Type baseTy = base->getType();
+      if (baseTy->is<ErrorType>())
+        return false;
+
+      Optional<InFlightDiagnostic> activeDiag;
+      if (auto *baseInit = dyn_cast<ConstructorDecl>(base)) {
+        // Special-case initializers, whose "type" isn't useful besides the
+        // input arguments.
+        baseTy = baseTy->getAs<AnyFunctionType>()->getResult();
+        Type argTy = baseTy->getAs<AnyFunctionType>()->getInput();
+        auto diagKind = diag::override_type_mismatch_with_fixits_init;
+        unsigned numArgs = baseInit->getParameters()->size();
+        activeDiag.emplace(TC.diagnose(decl, diagKind,
+                                       /*plural*/std::min(numArgs, 2U),
+                                       argTy));
+      } else {
+        if (isa<AbstractFunctionDecl>(base))
+          baseTy = baseTy->getAs<AnyFunctionType>()->getResult();
+
+        activeDiag.emplace(TC.diagnose(decl,
+                                       diag::override_type_mismatch_with_fixits,
+                                       base->getDescriptiveKind(), baseTy));
+      }
+
+      if (fixOverrideDeclarationTypes(*activeDiag, TC, decl, base))
+        return true;
+    }
+
+    // There weren't any fixes we knew how to make. Drop this diagnostic.
+    tentativeDiags.abort();
+    return false;
+  }
+
   enum class OverrideCheckingAttempt {
     PerfectMatch,
     MismatchedOptional,
@@ -5132,9 +5284,21 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
         // Nothing to do.
 
       } else if (method) {
-        // Private migration help for overrides of Objective-C methods.
-        if ((!isa<FuncDecl>(method) || !cast<FuncDecl>(method)->isAccessor()) &&
-            (matchDecl->isObjC() || mayHaveMismatchedOptionals)) {
+        if (attempt == OverrideCheckingAttempt::MismatchedTypes) {
+          auto diagKind = diag::method_does_not_override;
+          if (ctor)
+            diagKind = diag::initializer_does_not_override;
+          TC.diagnose(decl, diagKind);
+          noteFixableMismatchedTypes(TC, decl, matchDecl);
+          TC.diagnose(matchDecl, diag::overridden_near_match_here,
+                      matchDecl->getDescriptiveKind(),
+                      matchDecl->getFullName());
+          emittedMatchError = true;
+
+        } else if ((!isa<FuncDecl>(method) ||
+                    !cast<FuncDecl>(method)->isAccessor()) &&
+                   (matchDecl->isObjC() || mayHaveMismatchedOptionals)) {
+          // Private migration help for overrides of Objective-C methods.
           TypeLoc resultTL;
           if (auto *methodAsFunc = dyn_cast<FuncDecl>(method))
             resultTL = methodAsFunc->getBodyResultTypeLoc();
@@ -5156,7 +5320,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
           return true;
         }
 
-        if (mayHaveMismatchedOptionals) {
+        if (attempt == OverrideCheckingAttempt::MismatchedTypes) {
+          TC.diagnose(decl, diag::subscript_does_not_override);
+          noteFixableMismatchedTypes(TC, decl, matchDecl);
+          TC.diagnose(matchDecl, diag::overridden_near_match_here,
+                      matchDecl->getDescriptiveKind(),
+                      matchDecl->getFullName());
+          emittedMatchError = true;
+
+        } else if (mayHaveMismatchedOptionals) {
           emittedMatchError |=
               diagnoseMismatchedOptionals(TC, subscript,
                                           subscript->getIndices(),
@@ -5174,6 +5346,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
                                      &TC)) {
           TC.diagnose(property, diag::override_property_type_mismatch,
                       property->getName(), propertyTy, parentPropertyTy);
+          noteFixableMismatchedTypes(TC, decl, matchDecl);
           TC.diagnose(matchDecl, diag::property_override_here);
           return true;
         }

test/ClangModules/objc_bridging_custom.swift

@@ -0,0 +1,87 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -emit-module -I %S/../Inputs/ObjCBridging %S/../Inputs/ObjCBridging/Appliances.swift -module-name Appliances -o %t
+
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -parse -I %S/../Inputs/ObjCBridging -I %t -parse-as-library -verify %s
+
+// REQUIRES: objc_interop
+
+import Appliances
+import Foundation
+
+func checkThatBridgingIsWorking(fridge: Refrigerator) {
+  let bridgedFridge = fridge as APPRefrigerator // no-warning
+  _ = bridgedFridge as Refrigerator // no-warning
+}
+
+class Base : NSObject {
+  func test(a: Refrigerator, b: Refrigerator) -> Refrigerator? { // expected-note {{potential overridden instance method 'test(a:b:)' here}}
+    return nil
+  }
+  func testGeneric(a: ManufacturerInfo<NSString>, b: ManufacturerInfo<NSString>) -> ManufacturerInfo<NSString>? { // expected-note {{potential overridden instance method 'testGeneric(a:b:)' here}} {{none}}
+    return nil
+  }
+  class func testInout(_: inout Refrigerator) {} // expected-note {{potential overridden class method 'testInout' here}}
+  func testUnmigrated(a: NSRuncingMode, b: Refrigerator, c: NSCoding) {} // expected-note {{potential overridden instance method 'testUnmigrated(a:b:c:)' here}}
+  func testPartialMigrated(a: NSRuncingMode, b: Refrigerator) {} // expected-note {{potential overridden instance method 'testPartialMigrated(a:b:)' here}}
+
+  subscript(a a: Refrigerator, b b: Refrigerator) -> Refrigerator? { // expected-note {{potential overridden subscript 'subscript(a:b:)' here}} {{none}}
+    return nil
+  }
+  subscript(generic a: ManufacturerInfo<NSString>, b b: ManufacturerInfo<NSString>) -> ManufacturerInfo<NSString>? { // expected-note {{potential overridden subscript 'subscript(generic:b:)' here}} {{none}}
+    return nil
+  }
+
+  init?(a: Refrigerator, b: Refrigerator) {} // expected-note {{potential overridden initializer 'init(a:b:)' here}} {{none}}
+  init?(generic: ManufacturerInfo<NSString>, b: ManufacturerInfo<NSString>) {} // expected-note {{potential overridden initializer 'init(generic:b:)' here}} {{none}}
+  init(singleArgument: Refrigerator) {} // expected-note {{potential overridden initializer 'init(singleArgument:)' here}} {{none}}
+
+  // FIXME: expected-note@+1 {{getter for 'prop' declared here}}
+  var prop: Refrigerator // expected-note {{attempt to override property here}}
+  // FIXME: expected-note@+1 {{getter for 'propGeneric' declared here}}
+  var propGeneric: ManufacturerInfo<NSString> // expected-note {{attempt to override property here}}
+}
+
+class Sub : Base {
+  // expected-note@+1 {{type does not match superclass instance method with type '(a: Refrigerator, b: Refrigerator) -> Refrigerator?'}} {{25-40=Refrigerator}} {{45-61=Refrigerator?}} {{66-81=Refrigerator}}
+  override func test(a: APPRefrigerator, b: APPRefrigerator?) -> APPRefrigerator { // expected-error {{method does not override any method from its superclass}} {{none}}
+    return a
+  }
+  // expected-note@+1 {{type does not match superclass instance method with type '(a: ManufacturerInfo<NSString>, b: ManufacturerInfo<NSString>) -> ManufacturerInfo<NSString>?'}} {{32-62=ManufacturerInfo<NSString>}} {{67-98=ManufacturerInfo<NSString>?}} {{103-133=ManufacturerInfo<NSString>}}
+  override func testGeneric(a: APPManufacturerInfo<AnyObject>, b: APPManufacturerInfo<AnyObject>?) -> APPManufacturerInfo<AnyObject> { // expected-error {{method does not override any method from its superclass}} {{none}}
+    return a
+  }
+  // expected-note@+1 {{type does not match superclass class method with type '(inout Refrigerator) -> ()'}} {{36-57=inout Refrigerator}}
+  override class func testInout(_: inout APPRefrigerator) {} // expected-error {{method does not override any method from its superclass}} {{none}}
+
+  override func testUnmigrated(a: NSObject, b: NSObject, c: NSObject) {} // expected-error {{method does not override any method from its superclass}} {{none}}
+
+  // expected-note@+1 {{type does not match superclass instance method with type '(a: NSRuncingMode, b: Refrigerator) -> ()'}} {{53-68=Refrigerator}}
+  override func testPartialMigrated(a: NSObject, b: APPRefrigerator) {} // expected-error {{method does not override any method from its superclass}} {{none}}
+
+  // expected-note@+1 {{type does not match superclass subscript with type '(a: Refrigerator, b: Refrigerator) -> Refrigerator?'}} {{27-42=Refrigerator}} {{49-65=Refrigerator?}} {{70-85=Refrigerator}}
+  override subscript(a a: APPRefrigerator, b b: APPRefrigerator?) -> APPRefrigerator { // expected-error {{subscript does not override any subscript from its superclass}} {{none}}
+    return a
+  }
+  // expected-note@+1 {{type does not match superclass subscript with type '(generic: ManufacturerInfo<NSString>, b: ManufacturerInfo<NSString>) -> ManufacturerInfo<NSString>?'}} {{33-63=ManufacturerInfo<NSString>}} {{70-101=ManufacturerInfo<NSString>?}} {{106-136=ManufacturerInfo<NSString>}}
+  override subscript(generic a: APPManufacturerInfo<AnyObject>, b b: APPManufacturerInfo<AnyObject>?) -> APPManufacturerInfo<AnyObject> { // expected-error {{subscript does not override any subscript from its superclass}} {{none}}
+    return a
+  }
+
+  // expected-note@+1 {{type does not match superclass initializer with arguments '(a: Refrigerator, b: Refrigerator)'}} {{20-35=Refrigerator}} {{40-56=Refrigerator?}}
+  override init(a: APPRefrigerator, b: APPRefrigerator?) {} // expected-error {{initializer does not override a designated initializer from its superclass}}
+  // expected-note@+1 {{type does not match superclass initializer with arguments '(generic: ManufacturerInfo<NSString>, b: ManufacturerInfo<NSString>)'}} {{28-58=ManufacturerInfo<NSString>}} {{63-94=ManufacturerInfo<NSString>?}}
+  override init(generic a: APPManufacturerInfo<AnyObject>, b: APPManufacturerInfo<AnyObject>?) {} // expected-error {{initializer does not override a designated initializer from its superclass}}
+  // expected-note@+1 {{type does not match superclass initializer with argument '(singleArgument: Refrigerator)'}} {{33-48=Refrigerator}}
+  override init(singleArgument: APPRefrigerator) {} // expected-error {{initializer does not override a designated initializer from its superclass}}
+
+  // FIXME: expected-error@+2 {{getter for 'prop' with Objective-C selector 'prop' conflicts with getter for 'prop' from superclass 'Base' with the same Objective-C selector}}
+  // expected-note@+1 {{type does not match superclass var with type 'Refrigerator'}} {{22-37=Refrigerator}}
+  override var prop: APPRefrigerator { // expected-error {{property 'prop' with type 'APPRefrigerator' cannot override a property with type 'Refrigerator'}}
+    return super.prop // implicitly bridged
+  }
+  // FIXME: expected-error@+2 {{getter for 'propGeneric' with Objective-C selector 'propGeneric' conflicts with getter for 'propGeneric' from superclass 'Base' with the same Objective-C selector}}
+  // expected-note@+1 {{type does not match superclass var with type 'ManufacturerInfo<NSString>'}} {{29-59=ManufacturerInfo<NSString>}}
+  override var propGeneric: APPManufacturerInfo<AnyObject> { // expected-error {{property 'propGeneric' with type 'APPManufacturerInfo<AnyObject>' cannot override a property with type 'ManufacturerInfo<NSString>'}}
+    return super.prop // expected-error {{return type}}
+  }
+}

test/Inputs/ObjCBridging/Appliances.apinotes

@@ -3,3 +3,5 @@ Name: Appliances
 Classes:
 - Name: APPRefrigerator
   SwiftBridge: Refrigerator
+- Name: APPManufacturerInfo
+  SwiftBridge: ManufacturerInfo

test/Inputs/ObjCBridging/Appliances.h

@@ -12,3 +12,8 @@
 @interface APPHouse : NSObject
 @property (nonatomic,nonnull,copy) APPRefrigerator *fridge;
 @end
+
+
+@interface APPManufacturerInfo <DataType> : NSObject
+@property (nonatomic,nonnull,readonly) DataType value;
+@end

test/Inputs/ObjCBridging/Appliances.m

@@ -22,5 +22,7 @@ -(instancetype)init {
   }
   return self;
 }
+@end
 
+@implementation APPManufacturerInfo
 @end