[4.2-04-30] Cherry-pick migrator changes

include/swift/IDE/APIDigesterData.h

@@ -247,6 +247,7 @@ enum class TypeMemberDiffItemSubKind {
   HoistSelfOnly,
   HoistSelfAndRemoveParam,
   HoistSelfAndUseProperty,
+  FuncRename,
 };
 
 struct TypeMemberDiffItem: public APIDiffItem {

lib/IDE/APIDigesterData.cpp

@@ -148,9 +148,10 @@ swift::ide::api::TypeMemberDiffItem::getSubKind() const {
     assert(OldName.argSize() == 0);
     assert(!removedIndex);
     return TypeMemberDiffItemSubKind::GlobalFuncToStaticProperty;
-  } else if (oldTypeName.empty()){
+  } else if (oldTypeName.empty()) {
+    // we can handle this as a simple function rename.
     assert(NewName.argSize() == OldName.argSize());
-    return TypeMemberDiffItemSubKind::SimpleReplacement;
+    return TypeMemberDiffItemSubKind::FuncRename;
   } else {
     assert(NewName.argSize() == OldName.argSize());
     return TypeMemberDiffItemSubKind::QualifiedReplacement;

lib/Migrator/APIDiffMigratorPass.cpp

@@ -247,6 +247,12 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
 
   APIDiffItemStore DiffStore;
 
+  bool isNilExpr(Expr *E) {
+    auto Range = E->getSourceRange();
+    return Range.isValid() && Lexer::getCharSourceRangeFromSourceRange(
+      SF->getASTContext().SourceMgr, Range).str() == "nil";
+  }
+
   std::vector<APIDiffItem*> getRelatedDiffItems(ValueDecl *VD) {
     std::vector<APIDiffItem*> results;
     auto addDiffItems = [&](ValueDecl *VD) {
@@ -266,7 +272,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     return results;
   }
 
-  DeclNameViewer getFuncRename(ValueDecl *VD, bool &IgnoreBase) {
+  DeclNameViewer getFuncRename(ValueDecl *VD, llvm::SmallString<32> &Buffer,
+                               bool &IgnoreBase) {
     for (auto *Item: getRelatedDiffItems(VD)) {
       if (auto *CI = dyn_cast<CommonDiffItem>(Item)) {
         if (CI->isRename()) {
@@ -282,6 +289,13 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
           }
         }
       }
+      if (auto *MI = dyn_cast<TypeMemberDiffItem>(Item)) {
+        if (MI->Subkind == TypeMemberDiffItemSubKind::FuncRename) {
+          llvm::raw_svector_ostream OS(Buffer);
+          OS << MI->newTypeName << "." << MI->newPrintedName;
+          return DeclNameViewer(OS.str());
+        }
+      }
     }
     return DeclNameViewer();
   }
@@ -413,7 +427,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
 
   void handleFuncRename(ValueDecl *FD, Expr* FuncRefContainer, Expr *Arg) {
     bool IgnoreBase = false;
-    if (auto View = getFuncRename(FD, IgnoreBase)) {
+    llvm::SmallString<32> Buffer;
+    if (auto View = getFuncRename(FD, Buffer, IgnoreBase)) {
       if (!IgnoreBase) {
         ReferenceCollector Walker(FD);
         Walker.walk(FuncRefContainer);
@@ -575,7 +590,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     if (!Item)
       return false;
     if (Item->Subkind == TypeMemberDiffItemSubKind::SimpleReplacement ||
-        Item->Subkind == TypeMemberDiffItemSubKind::QualifiedReplacement)
+        Item->Subkind == TypeMemberDiffItemSubKind::QualifiedReplacement ||
+        Item->Subkind == TypeMemberDiffItemSubKind::FuncRename)
       return false;
 
     if (Item->Subkind == TypeMemberDiffItemSubKind::GlobalFuncToStaticProperty) {
@@ -624,6 +640,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     }
 
     switch (Item->Subkind) {
+    case TypeMemberDiffItemSubKind::FuncRename:
     case TypeMemberDiffItemSubKind::GlobalFuncToStaticProperty:
     case TypeMemberDiffItemSubKind::SimpleReplacement:
     case TypeMemberDiffItemSubKind::QualifiedReplacement:
@@ -696,24 +713,27 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     auto *RD = getReferencedDecl(Reference);
     if (!RD)
       return false;
-    std::string Func;
     std::string Rename;
+    Optional<NodeAnnotation> Kind;
+    StringRef RightComment;
     for (auto *Item: getRelatedDiffItems(RD)) {
       if (isSimpleReplacement(Item, Rename)) {
       } else if (auto *CI = dyn_cast<CommonDiffItem>(Item)) {
         if (CI->isStringRepresentableChange() &&
             CI->NodeKind == SDKNodeKind::DeclVar) {
-          SmallString<256> Buffer;
-          Func = insertHelperFunction(CI->DiffKind, CI->RightComment, Buffer,
-            FromString);
+          Kind = CI->DiffKind;
+          RightComment = CI->RightComment;
         }
       }
     }
-    if (Func.empty())
+    if (!Kind.hasValue())
       return false;
-
-    Editor.insert(WrapperTarget->getStartLoc(), (Twine(Func) + "(").str());
-    Editor.insertAfterToken(WrapperTarget->getEndLoc(), ")");
+    if (Kind && !isNilExpr(WrapperTarget)) {
+      SmallString<256> Buffer;
+      auto Func = insertHelperFunction(*Kind, RightComment, Buffer, FromString);
+      Editor.insert(WrapperTarget->getStartLoc(), (Twine(Func) + "(").str());
+      Editor.insertAfterToken(WrapperTarget->getEndLoc(), ")");
+    }
     if (!Rename.empty()) {
       replaceExpr(Reference, Rename);
     }
@@ -838,21 +858,25 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
     }
     if (NewAttributeType.empty())
       return;
-    SmallString<256> Buffer;
-    auto FuncName = insertHelperFunction(Kind, NewAttributeType, Buffer,
-                                         /*FromString*/ArgIdx);
+    Expr *WrapTarget = Call;
+    bool FromString = false;
     if (ArgIdx) {
       ArgIdx --;
+      FromString = true;
       auto AllArgs = getCallArgInfo(SM, Arg, LabelRangeEndAt::LabelNameOnly);
       if (AllArgs.size() <= ArgIdx)
         return;
-      auto Exp = AllArgs[ArgIdx].ArgExp;
-      Editor.insert(Exp->getStartLoc(), (Twine(FuncName) + "(").str());
-      Editor.insertAfterToken(Exp->getEndLoc(), ")");
-    } else {
-      Editor.insert(Call->getStartLoc(), (Twine(FuncName) + "(").str());
-      Editor.insertAfterToken(Call->getEndLoc(), ")");
+      WrapTarget = AllArgs[ArgIdx].ArgExp;
+      // Avoid wrapping nil literal.
+      if (isNilExpr(WrapTarget))
+        return;
     }
+    assert(WrapTarget);
+    SmallString<256> Buffer;
+    auto FuncName = insertHelperFunction(Kind, NewAttributeType, Buffer,
+                                         FromString);
+    Editor.insert(WrapTarget->getStartLoc(), (Twine(FuncName) + "(").str());
+    Editor.insertAfterToken(WrapTarget->getEndLoc(), ")");
   }
 
   bool walkToExprPre(Expr *E) override {
@@ -904,7 +928,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
   void handleFuncDeclRename(AbstractFunctionDecl *AFD,
                             CharSourceRange NameRange) {
     bool IgnoreBase = false;
-    if (auto View = getFuncRename(AFD, IgnoreBase)) {
+    llvm::SmallString<32> Buffer;
+    if (auto View = getFuncRename(AFD, Buffer, IgnoreBase)) {
       if (!IgnoreBase)
         Editor.replace(NameRange, View.base());
       unsigned Index = 0;

test/Migrator/Inputs/API.json

@@ -509,5 +509,13 @@
     "RightUsr": "",
     "RightComment": "BarBase.Nested",
     "ModuleName": "bar"
-  }
+  },
+  {
+    "DiffItemKind": "TypeMemberDiffItem",
+    "Usr": "c:@F@barGlobalHoistedFuncOldName",
+    "OldPrintedName": "barGlobalHoistedFuncOldName(_:_:_:)",
+    "OldTypeName": "",
+    "NewPrintedName": "newName(is:at:for:)",
+    "NewTypeName": "AwesomeWrapper"
+  },
 ]

test/Migrator/mock-sdk/Bar.framework/Headers/Bar.h

@@ -10,6 +10,8 @@ int barGlobalVariableOldEnumElement = 1;
 
 int barGlobalFuncOldName(int a);
 
+int barGlobalHoistedFuncOldName(int a, int b, int c);
+
 @interface BarForwardDeclaredClass
 - (id _Nonnull)initWithOldLabel0:(int)frame;
 - (void) barInstanceFunc0;

test/Migrator/rename.swift

@@ -18,6 +18,7 @@ func foo(_ b: BarForwardDeclaredClass) {
   _ = barGlobalVariableOldEnumElement
   _ = PropertyUserInterface.methodPlus()
   let _: BarBaseNested
+  _ = barGlobalHoistedFuncOldName(0, 1, 2)
 }
 
 func foo1(_ b: BarForwardDeclaredClass) {

test/Migrator/rename.swift.expected

@@ -18,6 +18,7 @@ func foo(_ b: BarForwardDeclaredClass) {
   _ = NewEnum.enumElement
   _ = PropertyUserInterface.newMethodPlus()
   let _: BarBase.Nested
+  _ = AwesomeWrapper.newName(is: 0, at: 1, for: 2)
 }
 
 func foo1(_ b: BarForwardDeclaredClass) {

test/Migrator/string-representable.swift

@@ -12,10 +12,13 @@ func foo(_ c: Container) -> String {
   c.addingAttributes(["a": "b", "a": "b", "a": "b"])
   c.adding(attributes: ["a": 1, "a": 2, "a": 3])
   c.adding(optionalAttributes: ["a": 1, "a": 2, "a": 3])
+  _ = Container(optionalAttributes: [:])
+  _ = Container(optionalAttrArray: [])
   _ = Container(optionalAttributes: nil)
   _ = Container(optionalAttrArray: nil)
   c.adding(attrArray: ["key1", "key2"])
   c.add(single: "")
+  c.add(singleOptional: "")
   c.add(singleOptional: nil)
   _ = c.getAttrDictionary()
   _ = c.getOptionalAttrDictionary()
@@ -36,5 +39,6 @@ func foo(_ c: Container) -> String {
   c.adding(optionalAttributes: c.optionalAttrDict)
   _ = GlobalAttribute
   c.Value = GlobalAttribute
+  c.optionalAttrDict = nil
   return c.Value
 }

test/Migrator/string-representable.swift.expected

@@ -12,11 +12,14 @@ func foo(_ c: Container) -> String {
   c.addingAttributes(convertToCitiesContainerAttributeDictionary(["a": "b", "a": "b", "a": "b"]))
   c.adding(attributes: convertToSimpleAttributeDictionary(["a": 1, "a": 2, "a": 3]))
   c.adding(optionalAttributes: convertToOptionalSimpleAttributeDictionary(["a": 1, "a": 2, "a": 3]))
-  _ = Container(optionalAttributes: convertToOptionalSimpleAttributeDictionary(nil))
-  _ = Container(optionalAttrArray: convertToOptionalSimpleAttributeArray(nil))
+  _ = Container(optionalAttributes: convertToOptionalSimpleAttributeDictionary([:]))
+  _ = Container(optionalAttrArray: convertToOptionalSimpleAttributeArray([]))
+  _ = Container(optionalAttributes: nil)
+  _ = Container(optionalAttrArray: nil)
   c.adding(attrArray: convertToSimpleAttributeArray(["key1", "key2"]))
   c.add(single: convertToSimpleAttribute(""))
-  c.add(singleOptional: convertToOptionalSimpleAttribute(nil))
+  c.add(singleOptional: convertToOptionalSimpleAttribute(""))
+  c.add(singleOptional: nil)
   _ = convertFromSimpleAttributeDictionary(c.getAttrDictionary())
   _ = convertFromOptionalSimpleAttributeDictionary(c.getOptionalAttrDictionary())
   _ = convertFromSimpleAttribute(c.getSingleAttr())
@@ -36,6 +39,7 @@ func foo(_ c: Container) -> String {
   c.adding(optionalAttributes: convertToOptionalSimpleAttributeDictionary(convertFromOptionalSimpleAttributeDictionary(c.optionalAttrDict)))
   _ = convertFromNewAttribute(AttributeWrapper.NewAttribute)
   c.Value = convertToNewAttribute(convertFromNewAttribute(AttributeWrapper.NewAttribute))
+  c.optionalAttrDict = nil
   return convertFromNewAttribute(c.Value)
 }
 

tools/swift-api-digester/swift-api-digester.cpp

@@ -622,7 +622,8 @@ void SDKNode::removeChild(NodePtr C) {
 }
 
 void SDKNode::annotate(NodeAnnotation Anno, StringRef Comment) {
-  assert(!isAnnotatedAs(Anno) && "already annotated");
+  assert(!Comment.empty());
+  assert(!isAnnotatedAs(Anno) || AnnotateComments[Anno] == Comment);
   annotate(Anno);
   AnnotateComments[Anno] = Comment;
 }
@@ -2357,8 +2358,8 @@ static void detectFuncDeclChange(NodePtr L, NodePtr R) {
 }
 
 static void detectRename(NodePtr L, NodePtr R) {
-  assert(L->getKind() == R->getKind());
-  if (isa<SDKNodeDecl>(L) && L->getPrintedName() != R->getPrintedName()) {
+  if (L->getKind() == R->getKind() && isa<SDKNodeDecl>(L) &&
+      L->getPrintedName() != R->getPrintedName()) {
     L->annotate(NodeAnnotation::Rename);
     L->annotate(NodeAnnotation::RenameOldName, L->getPrintedName());
     L->annotate(NodeAnnotation::RenameNewName, R->getPrintedName());