[4.2] migrator: handle changed parameter declarations by introducing bridging local variables.

include/swift/AST/Decl.h

@@ -4830,6 +4830,9 @@ class ParamDecl : public VarDecl {
   /// Retrieve the argument (API) name for this function parameter.
   Identifier getArgumentName() const { return ArgumentName; }
 
+  /// Retrieve the parameter (local) name for this function parameter.
+  Identifier getParameterName() const { return getName(); }
+
   /// Retrieve the source location of the argument (API) name.
   ///
   /// The resulting source location will be valid if the argument name

lib/Migrator/APIDiffMigratorPass.cpp

@@ -404,7 +404,27 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
         InsertedFunctions.insert(FD->getBaseName().getIdentifier().str());
       }
     }
+
+    // Handle helper functions without wrappees first.
+    for (auto &Cur: HelperFuncInfo) {
+      if (Cur.ExpressionToWrap)
+        continue;
+      auto FuncName = Cur.getFuncName();
+      // Avoid inserting the helper function if it's already present.
+      if (!InsertedFunctions.count(FuncName)) {
+        Editor.insert(FileEndLoc, Cur.getFuncDef());
+        InsertedFunctions.insert(FuncName);
+      }
+    }
+
+    // Remove all helper functions that're without expressions to wrap.
+    HelperFuncInfo.erase(std::remove_if(HelperFuncInfo.begin(),
+      HelperFuncInfo.end(), [](const ConversionFunctionInfo& Info) {
+        return !Info.ExpressionToWrap;
+    }), HelperFuncInfo.end());
+
     for (auto &Cur: HelperFuncInfo) {
+      assert(Cur.ExpressionToWrap);
       // Avoid wrapping nil expression.
       if (isNilExpr(Cur.ExpressionToWrap))
         continue;
@@ -885,9 +905,11 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     return wrapAttributeReference(E, E, false);
   }
 
-  void insertHelperFunction(NodeAnnotation Anno, StringRef RawType,
-                            StringRef NewType, bool FromString,
-                            Expr *Wrappee) {
+  ConversionFunctionInfo &insertHelperFunction(NodeAnnotation Anno,
+                                               StringRef RawType,
+                                               StringRef NewType,
+                                               bool FromString,
+                                               Expr *Wrappee) {
     HelperFuncInfo.emplace_back(Wrappee);
     ConversionFunctionInfo &Info = HelperFuncInfo.back();
     llvm::raw_svector_ostream OS(Info.Buffer);
@@ -959,6 +981,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
       OS << "(_ input: " << Segs[3] << ") -> " << Segs[2] << " {\n";
       OS << Segs[5] << "\n}\n";
     }
+    return Info;
   }
 
   void handleStringRepresentableArg(ValueDecl *FD, Expr *Arg, Expr *Call) {
@@ -1106,6 +1129,18 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     return true;
   }
 
+  static void collectParamters(AbstractFunctionDecl *AFD,
+                               SmallVectorImpl<ParamDecl*> &Results) {
+    for (auto PL : AFD->getParameterLists()) {
+      for (auto *PD: *PL) {
+        // Self parameter should not be updated.
+        if (PD->isSelfParameter())
+          continue;
+        Results.push_back(PD);
+      }
+    }
+  }
+
   void handleFuncDeclRename(AbstractFunctionDecl *AFD,
                             CharSourceRange NameRange) {
     bool IgnoreBase = false;
@@ -1114,29 +1149,26 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
       if (!IgnoreBase)
         Editor.replace(NameRange, View.base());
       unsigned Index = 0;
-      for (auto PL : AFD->getParameterLists()) {
-        for (auto *PD : *PL) {
-          if (Index == View.argSize())
-            break;
-          // Self parameter should not be updated.
-          if (PD->isSelfParameter())
-            continue;
-          StringRef NewArg = View.args()[Index++];
-          auto ArgLoc = PD->getArgumentNameLoc();
-
-          // Represent empty label with underscore.
-          if (NewArg.empty())
-            NewArg = "_";
-
-          // If the argument name is not specified, add the argument name before
-          // the parameter name.
-          if (ArgLoc.isInvalid())
-            Editor.insertBefore(PD->getNameLoc(),
-                                (llvm::Twine(NewArg) + " ").str());
-          else {
-            // Otherwise, replace the argument name directly.
-            Editor.replaceToken(ArgLoc, NewArg);
-          }
+      SmallVector<ParamDecl*, 4> Params;
+      collectParamters(AFD, Params);
+      for (auto *PD: Params) {
+        if (Index == View.argSize())
+          break;
+        StringRef NewArg = View.args()[Index++];
+        auto ArgLoc = PD->getArgumentNameLoc();
+
+        // Represent empty label with underscore.
+        if (NewArg.empty())
+          NewArg = "_";
+
+        // If the argument name is not specified, add the argument name before
+        // the parameter name.
+        if (ArgLoc.isInvalid())
+          Editor.insertBefore(PD->getNameLoc(),
+                              (llvm::Twine(NewArg) + " ").str());
+        else {
+          // Otherwise, replace the argument name directly.
+          Editor.replaceToken(ArgLoc, NewArg);
         }
       }
     }
@@ -1224,6 +1256,49 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     }
   }
 
+  // When users override a SDK function whose parameter types have been changed,
+  // we should introduce a local variable in the body of the function definition
+  // to shadow the changed parameter. Also, a proper conversion function should
+  // be defined to bridge the parameter to the local variable.
+  void handleLocalParameterBridge(AbstractFunctionDecl *AFD,
+                                  CommonDiffItem *DiffItem) {
+    assert(AFD);
+    assert(DiffItem->isStringRepresentableChange());
+
+    // We only handle top-level parameter type change.
+    if (DiffItem->getChildIndices().size() != 1)
+      return;
+    auto Idx = DiffItem->getChildIndices().front();
+
+    // We don't handle return type change.
+    if (Idx == 0)
+      return;
+    Idx --;
+    SmallVector<ParamDecl*, 4> Params;
+    collectParamters(AFD, Params);
+    if (Params.size() <= Idx)
+      return;
+
+    // Get the internal name of the changed paramter.
+    auto VariableName = Params[Idx]->getParameterName().str();
+
+    // Insert the helper function to convert the type back to raw types.
+    auto &Info = insertHelperFunction(DiffItem->DiffKind, DiffItem->LeftComment,
+      DiffItem->RightComment, /*From String*/false,
+      /*No expression to wrap*/nullptr);
+
+    if (auto *BD = AFD->getBody()) {
+      auto BL = BD->getLBraceLoc();
+      if (BL.isValid()) {
+        // Insert the local variable declaration after the opening brace.
+        Editor.insertAfterToken(BL,
+          (llvm::Twine("\n// Local variable inserted by Swift 4.2 migrator.") +
+           "\nlet " + VariableName + " = " + Info.getFuncName() + "(" +
+           VariableName + ")\n").str());
+      }
+    }
+  }
+
   bool walkToDeclPre(Decl *D, CharSourceRange Range) override {
     if (D->isImplicit())
       return true;
@@ -1235,6 +1310,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
             handleOverridingTypeChange(AFD, DiffItem);
           else if (DiffItem->isToPropertyChange())
             handleOverridingPropertyChange(AFD, DiffItem);
+          else if (DiffItem->isStringRepresentableChange())
+            handleLocalParameterBridge(AFD, DiffItem);
         }
       }
     }

test/Migrator/Inputs/Cities.swift

@@ -35,25 +35,26 @@ public func globalCityFunc4(_ c : Cities, _ p : Int, _ q: Int) -> Int { return 0
 public func globalCityFunc5() -> Int { return 0 }
 public func globalCityPointerTaker(_ c : UnsafePointer<Cities>, _ p : Int, _ q: Int) -> Int { return 0 }
 
-public class Container {
-  public var Value: String = ""
-  public var attrDict: [String: Any] = [:]
-  public var attrArr: [String] = []
-  public var optionalAttrDict: [String: Any]? = nil
-  public func addingAttributes(_ input: [String: Any]) {}
-  public func adding(attributes: [String: Any]) {}
-  public func adding(optionalAttributes: [String: Any]?) {}
+open class Container {
   public init(optionalAttributes: [String: Any]?) {}
-  public func adding(attrArray: [String]) {}
   public init(optionalAttrArray: [String]?) {}
-  public func add(single: String) {}
-  public func add(singleOptional: String?) {}
-  public func getAttrArray() -> [String] { return [] }
-  public func getOptionalAttrArray() -> [String]? { return [] }
-  public func getAttrDictionary() -> [String: Any] { return [:] }
-  public func getOptionalAttrDictionary() -> [String: Any]? { return nil }
-  public func getSingleAttr() -> String { return "" }
-  public func getOptionalSingleAttr() -> String? { return nil }
+
+  open func adding(attrArray: [String]) {}
+  open var Value: String = ""
+  open var attrDict: [String: Any] = [:]
+  open var attrArr: [String] = []
+  open var optionalAttrDict: [String: Any]? = nil
+  open func addingAttributes(_ input: [String: Any]) {}
+  open func adding(attributes: [String: Any]) {}
+  open func adding(optionalAttributes: [String: Any]?) {}
+  open func add(single: String) {}
+  open func add(singleOptional: String?) {}
+  open func getAttrArray() -> [String] { return [] }
+  open func getOptionalAttrArray() -> [String]? { return [] }
+  open func getAttrDictionary() -> [String: Any] { return [:] }
+  open func getOptionalAttrDictionary() -> [String: Any]? { return nil }
+  open func getSingleAttr() -> String { return "" }
+  open func getOptionalSingleAttr() -> String? { return nil }
 }
 
 open class ToplevelType {

test/Migrator/string-representable.swift

@@ -65,3 +65,11 @@ func bar(_ c: Container) {
   let attr: AliasAttribute = ""
   c.add(single: attr)
 }
+
+public class SubContainer: Container {
+  public override func adding(optionalAttributes subname: [String: Any]?) {}
+  public override func adding(attributes myname: [String: Any]) {}
+  public override func adding(attrArray: [String]) {}
+  public override func add(single: String) {}
+  public override func add(singleOptional: String?) {}
+}

test/Migrator/string-representable.swift.expected

@@ -66,6 +66,56 @@ func bar(_ c: Container) {
   c.add(single: attr)
 }
 
+public class SubContainer: Container {
+  public override func adding(optionalAttributes subname: [String: Any]?) {
+// Local variable inserted by Swift 4.2 migrator.
+let subname = convertFromOptionalSimpleAttributeDictionary(subname)
+}
+  public override func adding(attributes myname: [String: Any]) {
+// Local variable inserted by Swift 4.2 migrator.
+let myname = convertFromSimpleAttributeDictionary(myname)
+}
+  public override func adding(attrArray: [String]) {
+// Local variable inserted by Swift 4.2 migrator.
+let attrArray = convertFromSimpleAttributeArray(attrArray)
+}
+  public override func add(single: String) {
+// Local variable inserted by Swift 4.2 migrator.
+let single = convertFromSimpleAttribute(single)
+}
+  public override func add(singleOptional: String?) {
+// Local variable inserted by Swift 4.2 migrator.
+let singleOptional = convertFromOptionalSimpleAttribute(singleOptional)
+}
+}
+
+// Helper function inserted by Swift 4.2 migrator.
+fileprivate func convertFromOptionalSimpleAttributeDictionary(_ input: [SimpleAttribute: Any]?) -> [String: Any]? {
+	guard let input = input else { return nil }
+	return Dictionary(uniqueKeysWithValues: input.map {key, value in (key.rawValue, value)})
+}
+
+// Helper function inserted by Swift 4.2 migrator.
+fileprivate func convertFromSimpleAttributeDictionary(_ input: [SimpleAttribute: Any]) -> [String: Any] {
+	return Dictionary(uniqueKeysWithValues: input.map {key, value in (key.rawValue, value)})
+}
+
+// Helper function inserted by Swift 4.2 migrator.
+fileprivate func convertFromSimpleAttributeArray(_ input: [SimpleAttribute]) -> [String] {
+	return input.map { key in key.rawValue }
+}
+
+// Helper function inserted by Swift 4.2 migrator.
+fileprivate func convertFromSimpleAttribute(_ input: SimpleAttribute) -> String {
+	return input.rawValue
+}
+
+// Helper function inserted by Swift 4.2 migrator.
+fileprivate func convertFromOptionalSimpleAttribute(_ input: SimpleAttribute?) -> String? {
+	guard let input = input else { return nil }
+	return input.rawValue
+}
+
 // Helper function inserted by Swift 4.2 migrator.
 fileprivate func convertToNewAttribute(_ input: String) -> NewAttribute {
 	return NewAttribute(rawValue: input)
@@ -109,33 +159,6 @@ fileprivate func convertToOptionalSimpleAttribute(_ input: String?) -> SimpleAtt
 	return SimpleAttribute(rawValue: input)
 }
 
-// Helper function inserted by Swift 4.2 migrator.
-fileprivate func convertFromSimpleAttributeDictionary(_ input: [SimpleAttribute: Any]) -> [String: Any] {
-	return Dictionary(uniqueKeysWithValues: input.map {key, value in (key.rawValue, value)})
-}
-
-// Helper function inserted by Swift 4.2 migrator.
-fileprivate func convertFromOptionalSimpleAttributeDictionary(_ input: [SimpleAttribute: Any]?) -> [String: Any]? {
-	guard let input = input else { return nil }
-	return Dictionary(uniqueKeysWithValues: input.map {key, value in (key.rawValue, value)})
-}
-
-// Helper function inserted by Swift 4.2 migrator.
-fileprivate func convertFromSimpleAttribute(_ input: SimpleAttribute) -> String {
-	return input.rawValue
-}
-
-// Helper function inserted by Swift 4.2 migrator.
-fileprivate func convertFromOptionalSimpleAttribute(_ input: SimpleAttribute?) -> String? {
-	guard let input = input else { return nil }
-	return input.rawValue
-}
-
-// Helper function inserted by Swift 4.2 migrator.
-fileprivate func convertFromSimpleAttributeArray(_ input: [SimpleAttribute]) -> [String] {
-	return input.map { key in key.rawValue }
-}
-
 // Helper function inserted by Swift 4.2 migrator.
 fileprivate func convertFromOptionalSimpleAttributeArray(_ input: [SimpleAttribute]?) -> [String]? {
 	guard let input = input else { return nil }