migrator: avoid inserting helper function call for nil expression. rdar://40173366

Author: nkcsgexi

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) {
@@ -696,24 +702,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 +847,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 {

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)
 }