[Migrator] Support framework API function renames. rdar://31766131 (#9013)

nkcsgexi · web-flow · commit 7163a40ffb78 · 2017-04-25T15:50:40.000-07:00
diff --git a/include/swift/IDE/Utils.h b/include/swift/IDE/Utils.h
@@ -300,8 +300,11 @@ class DeclNameViewer {
   SmallVector<StringRef, 4> Labels;
 public:
   DeclNameViewer(StringRef Text);
+  DeclNameViewer() : DeclNameViewer(StringRef()) {}
+  operator bool() const { return !BaseName.empty(); }
   StringRef base() const { return BaseName; }
   llvm::ArrayRef<StringRef> args() const { return llvm::makeArrayRef(Labels); }
+  unsigned argSize() const { return Labels.size(); }
   unsigned partsCount() const { return 1 + Labels.size(); }
   unsigned commonPartsCount(DeclNameViewer &Other) const;
 };
@@ -398,8 +401,15 @@ class SourceEditOutputConsumer : public SourceEditConsumer {
   void accept(SourceManager &SM, RegionType RegionType, ArrayRef<Replacement> Replacements) override;
 };
 
+enum class LabelRangeEndAt: int8_t {
+  BeforeElemStart,
+  LabelNameOnly,
+};
+
 // Get the ranges of argument labels from an Arg, either tuple or paren.
-std::vector<CharSourceRange> getCallArgLabelRanges(SourceManager &SM, Expr *Arg);
+std::vector<CharSourceRange>
+getCallArgLabelRanges(SourceManager &SM, Expr *Arg, LabelRangeEndAt EndKind);
+
 } // namespace ide
 } // namespace swift
 
diff --git a/lib/IDE/APIDigesterData.cpp b/lib/IDE/APIDigesterData.cpp
@@ -403,7 +403,7 @@ struct swift::ide::api::APIDiffItemStore::Implementation {
 ArrayRef<APIDiffItem*> swift::ide::api::APIDiffItemStore::
 getDiffItems(StringRef Key) const {
   if (Impl.PrintUsr)
-    llvm::outs() << Key;
+    llvm::outs() << Key << "\n";
   return Impl.Data[Key];
 }
 
diff --git a/lib/IDE/SwiftSourceDocInfo.cpp b/lib/IDE/SwiftSourceDocInfo.cpp
@@ -914,7 +914,7 @@ void swift::ide::getLocationInfo(const ValueDecl *VD,
 }
 
 std::vector<CharSourceRange> swift::ide::
-getCallArgLabelRanges(SourceManager &SM, Expr *Arg) {
+getCallArgLabelRanges(SourceManager &SM, Expr *Arg, LabelRangeEndAt EndKind) {
   std::vector<CharSourceRange> Ranges;
   if (TupleExpr *TE = dyn_cast<TupleExpr>(Arg)) {
     size_t ElemIndex = 0;
@@ -925,7 +925,10 @@ getCallArgLabelRanges(SourceManager &SM, Expr *Arg) {
       auto NameIdentifier = TE->getElementName(ElemIndex);
       if (!NameIdentifier.empty()) {
         LabelStart = TE->getElementNameLoc(ElemIndex);
+        if (EndKind == LabelRangeEndAt::LabelNameOnly)
+          LabelEnd = LabelStart.getAdvancedLoc(NameIdentifier.getLength());
       }
+
       Ranges.push_back(CharSourceRange(SM, LabelStart, LabelEnd));
       ++ElemIndex;
     }
diff --git a/lib/Migrator/SyntacticMigratorPass.cpp b/lib/Migrator/SyntacticMigratorPass.cpp
@@ -12,6 +12,7 @@
 
 #include "swift/AST/USRGeneration.h"
 #include "swift/Frontend/Frontend.h"
+#include "swift/IDE/Utils.h"
 #include "swift/Migrator/EditorAdapter.h"
 #include "swift/Migrator/FixitApplyDiagnosticConsumer.h"
 #include "swift/Migrator/Migrator.h"
@@ -27,6 +28,7 @@
 
 using namespace swift;
 using namespace swift::migrator;
+using namespace swift::ide;
 using namespace swift::ide::api;
 
 struct SyntacticMigratorPass::Implementation : public SourceEntityWalker {
@@ -48,6 +50,26 @@ struct SyntacticMigratorPass::Implementation : public SourceEntityWalker {
     return DiffStore.getDiffItems(Buffer.str());
   }
 
+  DeclNameViewer getFuncRename(ValueDecl *VD, bool &IgnoreBase) {
+    for (auto *Item : getRelatedDiffItems(VD)) {
+      if (auto *CI = dyn_cast<CommonDiffItem>(Item)) {
+        if (CI->isRename()) {
+          IgnoreBase = true;
+          switch(CI->NodeKind) {
+          case SDKNodeKind::Function:
+            IgnoreBase = false;
+            LLVM_FALLTHROUGH;
+          case SDKNodeKind::Constructor:
+            return DeclNameViewer(CI->getNewName());
+          default:
+            return DeclNameViewer();
+          }
+        }
+      }
+    }
+    return DeclNameViewer();
+  }
+
   bool isSimpleReplacement(APIDiffItem *Item, std::string &Text) {
     if (auto *MD = dyn_cast<TypeMemberDiffItem>(Item)) {
       // We need to pull the self if self index is set.
@@ -92,6 +114,72 @@ struct SyntacticMigratorPass::Implementation : public SourceEntityWalker {
     }
     return true;
   }
+
+  struct ReferenceCollector : public SourceEntityWalker {
+    ValueDecl *Target;
+    CharSourceRange Result;
+    ReferenceCollector(ValueDecl* Target) : Target(Target) {}
+    bool visitDeclReference(ValueDecl *D, CharSourceRange Range,
+                            TypeDecl *CtorTyRef, ExtensionDecl *ExtTyRef,
+                            Type T, ReferenceMetaData Data) override {
+      if (D == Target) {
+        Result = Range;
+        return false;
+      }
+      return true;
+    }
+  };
+
+  void handleFuncRename(ValueDecl *FD, Expr* FuncRefContainer, Expr *Arg) {
+    bool IgnoreBase = false;
+    if (auto View = getFuncRename(FD, IgnoreBase)) {
+      if(!IgnoreBase) {
+        ReferenceCollector Walker(FD);
+        Walker.walk(FuncRefContainer);
+        Editor.replace(Walker.Result, View.base());
+      }
+      unsigned Idx = 0;
+      for (auto LR :getCallArgLabelRanges(SM, Arg,
+                                          LabelRangeEndAt::LabelNameOnly)) {
+        if (Idx < View.argSize()) {
+          auto Label = View.args()[Idx++];
+
+          // FIXME: We update only when args are consistently valid.
+          if (Label != "_" && LR.getByteLength())
+            Editor.replace(LR, Label);
+        }
+      }
+    }
+  }
+
+  bool walkToExprPre(Expr *E) override {
+    if (auto *CE = dyn_cast<CallExpr>(E)) {
+      auto Fn = CE->getFn();
+      auto Args = CE->getArg();
+      switch (Fn->getKind()) {
+      case ExprKind::DeclRef: {
+        if (auto FD = Fn->getReferencedDecl().getDecl())
+          handleFuncRename(FD, Fn, Args);
+        break;
+      }
+      case ExprKind::DotSyntaxCall: {
+        auto DSC = cast<DotSyntaxCallExpr>(Fn);
+        if (auto FD = DSC->getFn()->getReferencedDecl().getDecl())
+          handleFuncRename(FD, DSC->getFn(), Args);
+        break;
+      }
+      case ExprKind::ConstructorRefCall: {
+        auto CCE = cast<ConstructorRefCallExpr>(Fn);
+        if (auto FD = CCE->getFn()->getReferencedDecl().getDecl())
+          handleFuncRename(FD, CCE->getFn(), Args);
+        break;
+      }
+      default:
+        break;
+      }
+    }
+    return true;
+  }
 };
 
 SyntacticMigratorPass::
diff --git a/test/migrator/API.json b/test/migrator/API.json
@@ -16,5 +16,38 @@
     "RightUsr": "",
     "RightComment": "theSimpleNewName",
     "ModuleName": "SomeModule"
+  },
+  {
+    "DiffItemKind": "CommonDiffItem",
+    "NodeKind": "Function",
+    "NodeAnnotation": "Rename",
+    "ChildIndex": "0",
+    "LeftUsr": "c:@F@barGlobalFuncOldName",
+    "LeftComment": "",
+    "RightUsr": "",
+    "RightComment": "barGlobalFuncNewName(newlabel:)",
+    "ModuleName": "bar"
+  },
+  {
+    "DiffItemKind": "CommonDiffItem",
+    "NodeKind": "Function",
+    "NodeAnnotation": "Rename",
+    "ChildIndex": "0",
+    "LeftUsr": "c:objc(cs)BarForwardDeclaredClass(im)barInstanceFunc1:anotherValue:anotherValue1:anotherValue2:",
+    "LeftComment": "",
+    "RightUsr": "",
+    "RightComment": "barNewInstanceFunc1(newlabel1:newlabel2:newlabel3:newlabel4:)",
+    "ModuleName": "bar"
+  },
+  {
+    "DiffItemKind": "CommonDiffItem",
+    "NodeKind": "Constructor",
+    "NodeAnnotation": "Rename",
+    "ChildIndex": "0",
+    "LeftUsr": "c:objc(cs)BarForwardDeclaredClass(im)initWithOldLabel0:",
+    "LeftComment": "",
+    "RightUsr": "",
+    "RightComment": "init(newlabel1:)",
+    "ModuleName": "bar"
   }
 ]
diff --git a/test/migrator/dump_usr.swift b/test/migrator/dump_usr.swift
diff --git a/test/migrator/mock-sdk/Bar.framework/Headers/Bar.h b/test/migrator/mock-sdk/Bar.framework/Headers/Bar.h
@@ -6,8 +6,12 @@ int barGlobalFunc(int a);
 
 int barGlobalVariable = 1;
 
+int barGlobalFuncOldName(int a);
+
 @interface BarForwardDeclaredClass
+- (id)initWithOldLabel0:(int)frame;
 - (void) barInstanceFunc0;
+- (void) barInstanceFunc1:(int)info anotherValue:(int)info1 anotherValue1:(int)info2 anotherValue2:(int)info3;
 @end
 
 enum BarForwardDeclaredEnum {
diff --git a/test/migrator/rename-init.swift b/test/migrator/rename-init.swift
@@ -0,0 +1,9 @@
+// REQUIRES: objc_interop
+// RUN: rm -rf %t && mkdir -p %t && %swift -update-code -primary-file %s  -F %S/mock-sdk -api-diff-data-file %S/API.json -emit-migrated-file-path %t/rename-init.swift.result -o %t/rename-init.swift.remap -disable-migrator-fixits
+// RUN: diff -u %S/rename-init.swift.expected %t/rename-init.swift.result
+
+import Bar
+
+func foo() {
+  _ = BarForwardDeclaredClass(oldLabel0:1)
+}
diff --git a/test/migrator/rename-init.swift.expected b/test/migrator/rename-init.swift.expected
@@ -0,0 +1,9 @@
+// REQUIRES: objc_interop
+// RUN: rm -rf %t && mkdir -p %t && %swift -update-code -primary-file %s  -F %S/mock-sdk -api-diff-data-file %S/API.json -emit-migrated-file-path %t/rename-init.swift.result -o %t/rename-init.swift.remap -disable-migrator-fixits
+// RUN: diff -u %S/rename-init.swift.expected %t/rename-init.swift.result
+
+import Bar
+
+func foo() {
+  _ = BarForwardDeclaredClass(newlabel1:1)
+}
diff --git a/test/migrator/rename.swift b/test/migrator/rename.swift
@@ -0,0 +1,18 @@
+// REQUIRES: objc_interop
+// RUN: rm -rf %t && mkdir -p %t && %swift -update-code -primary-file %s  -F %S/mock-sdk -api-diff-data-file %S/API.json -emit-migrated-file-path %t/rename.swift.result -o %t/rename.swift.remap
+// RUN: diff -u %S/rename.swift.expected %t/rename.swift.result
+
+import Bar
+
+func foo(_ b: BarForwardDeclaredClass) {
+  b.barInstanceFunc1(_: 0, anotherValue: 1, anotherValue1: 2, anotherValue2: 3)
+  barGlobalFuncOldName(2)
+  b.barInstanceFunc1(_: 0, anotherValue: 1, anotherValue1: 2, anotherValue2: 3)
+  barGlobalFuncOldName(2)
+  b.barInstanceFunc1(_: 0, anotherValue: 1, anotherValue1: 2, anotherValue2: 3)
+  barGlobalFuncOldName(2)
+  b.barInstanceFunc1(_: 0, anotherValue: 1, anotherValue1: 2, anotherValue2: 3)
+  barGlobalFuncOldName(2)
+  b.barInstanceFunc1(_: 0, anotherValue: 1, anotherValue1: 2, anotherValue2: 3)
+  barGlobalFuncOldName(2)
+}
diff --git a/test/migrator/rename.swift.expected b/test/migrator/rename.swift.expected
@@ -0,0 +1,18 @@
+// REQUIRES: objc_interop
+// RUN: rm -rf %t && mkdir -p %t && %swift -update-code -primary-file %s  -F %S/mock-sdk -api-diff-data-file %S/API.json -emit-migrated-file-path %t/rename.swift.result -o %t/rename.swift.remap
+// RUN: diff -u %S/rename.swift.expected %t/rename.swift.result
+
+import Bar
+
+func foo(_ b: BarForwardDeclaredClass) {
+  b.barNewInstanceFunc1(_: 0, newlabel2: 1, newlabel3: 2, newlabel4: 3)
+  barGlobalFuncNewName(2)
+  b.barNewInstanceFunc1(_: 0, newlabel2: 1, newlabel3: 2, newlabel4: 3)
+  barGlobalFuncNewName(2)
+  b.barNewInstanceFunc1(_: 0, newlabel2: 1, newlabel3: 2, newlabel4: 3)
+  barGlobalFuncNewName(2)
+  b.barNewInstanceFunc1(_: 0, newlabel2: 1, newlabel3: 2, newlabel4: 3)
+  barGlobalFuncNewName(2)
+  b.barNewInstanceFunc1(_: 0, newlabel2: 1, newlabel3: 2, newlabel4: 3)
+  barGlobalFuncNewName(2)
+}

Original file line number	Diff line number	Diff line change
`@@ -403,7 +403,7 @@ struct swift::ide::api::APIDiffItemStore::Implementation {`
`403`	`403`	`ArrayRef<APIDiffItem*> swift::ide::api::APIDiffItemStore::`
`404`	`404`	`getDiffItems(StringRef Key) const {`
`405`	`405`	`if (Impl.PrintUsr)`
`406`		`- llvm::outs() << Key;`
	`406`	`+ llvm::outs() << Key << "\n";`
`407`	`407`	`return Impl.Data[Key];`
`408`	`408`	`}`
`409`	`409`
Original file line number	Diff line number	Diff line change
`@@ -914,7 +914,7 @@ void swift::ide::getLocationInfo(const ValueDecl *VD,`
`914`	`914`	`}`
`915`	`915`
`916`	`916`	`std::vector<CharSourceRange> swift::ide::`
`917`		`-getCallArgLabelRanges(SourceManager &SM, Expr *Arg) {`
	`917`	`+getCallArgLabelRanges(SourceManager &SM, Expr *Arg, LabelRangeEndAt EndKind) {`
`918`	`918`	`std::vector<CharSourceRange> Ranges;`
`919`	`919`	`if (TupleExpr *TE = dyn_cast<TupleExpr>(Arg)) {`
`920`	`920`	`size_t ElemIndex = 0;`
`@@ -925,7 +925,10 @@ getCallArgLabelRanges(SourceManager &SM, Expr *Arg) {`
`925`	`925`	`auto NameIdentifier = TE->getElementName(ElemIndex);`
`926`	`926`	`if (!NameIdentifier.empty()) {`
`927`	`927`	`LabelStart = TE->getElementNameLoc(ElemIndex);`
	`928`	`+ if (EndKind == LabelRangeEndAt::LabelNameOnly)`
	`929`	`+ LabelEnd = LabelStart.getAdvancedLoc(NameIdentifier.getLength());`
`928`	`930`	`}`
	`931`	`+`
`929`	`932`	`Ranges.push_back(CharSourceRange(SM, LabelStart, LabelEnd));`
`930`	`933`	`++ElemIndex;`
`931`	`934`	`}`