[SourceKit] Merge local refactoring implementations

Retrieving local rename ranges and the local rename refactoring both had
almost identical methods, except for the addition of retrieving the
outermost shadowed decl that was added a couple months back. Merge them.

Resolves rdar://106529370.

include/swift/AST/DiagnosticsRefactoring.def

@@ -32,11 +32,19 @@ ERROR(arity_mismatch, none, "the given new name '%0' does not match the arity of
 
 ERROR(name_not_functionlike, none, "the 'call' name usage cannot be used with a non-function-like name '%0'", (StringRef))
 
-ERROR(unresolved_location, none, "cannot resolve location as name", ())
+ERROR(unresolved_location, none, "cannot rename due to unresolved location", ())
 
 ERROR(location_module_mismatch, none, "given location does not belong to module '%0'", (StringRef))
 
-ERROR(value_decl_no_loc, none, "value decl '%0' has no declaration location", (DeclName))
+ERROR(value_decl_no_loc, none, "cannot rename %0 as it has no declaration location", (DeclName))
+
+ERROR(decl_is_system_symbol, none, "cannot rename system symbol %0", (DeclName))
+
+ERROR(decl_has_no_name, none, "cannot rename as no declaration name was found", ())
+
+ERROR(decl_no_accessibility, none, "cannot rename as accessibility could not be determined", ())
+
+ERROR(decl_from_clang, none, "cannot rename a Clang symbol from its Swift reference", ())
 
 ERROR(value_decl_referenced_out_of_range, none, "value decl '%0' is referenced out of range", (DeclName))
 

include/swift/Refactoring/Refactoring.h

@@ -148,9 +148,8 @@ struct RenameRefInfo {
   bool IsArgLabel; ///< Whether Loc is on an arg label, rather than base name.
 };
 
-void collectRenameAvailabilityInfo(
-    const ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
-    llvm::SmallVectorImpl<RenameAvailabilityInfo> &Infos);
+Optional<RenameAvailabilityInfo>
+renameAvailabilityInfo(const ValueDecl *VD, Optional<RenameRefInfo> RefInfo);
 
 } // namespace ide
 } // namespace swift

lib/Refactoring/Refactoring.cpp

@@ -616,29 +616,54 @@ static const ValueDecl *getRelatedSystemDecl(const ValueDecl *VD) {
   return nullptr;
 }
 
-static Optional<RefactoringKind>
-getAvailableRenameForDecl(const ValueDecl *VD,
-                          Optional<RenameRefInfo> RefInfo) {
-  SmallVector<RenameAvailabilityInfo, 2> Infos;
-  collectRenameAvailabilityInfo(VD, RefInfo, Infos);
-  for (auto &Info : Infos) {
-    if (Info.AvailableKind == RenameAvailableKind::Available)
-      return Info.Kind;
-  }
+struct RenameInfo {
+  ValueDecl *VD;
+  RenameAvailabilityInfo Availability;
+};
+
+/// Given a cursor, return the decl and its rename availability. \c None if
+/// the cursor did not resolve to a decl or it resolved to a decl that we do
+/// not allow renaming on.
+static Optional<RenameInfo> getRenameInfo(ResolvedCursorInfoPtr cursorInfo) {
+  auto valueCursor = dyn_cast<ResolvedValueRefCursorInfo>(cursorInfo);
+  if (!valueCursor)
     return None;
+
+  ValueDecl *VD = valueCursor->typeOrValue();
+  if (!VD)
+    return None;
+
+  Optional<RenameRefInfo> refInfo;
+  if (!valueCursor->getShorthandShadowedDecls().empty()) {
+    // Find the outermost decl for a shorthand if let/closure capture
+    VD = valueCursor->getShorthandShadowedDecls().back();
+  } else if (valueCursor->isRef()) {
+    refInfo = {valueCursor->getSourceFile(), valueCursor->getLoc(),
+               valueCursor->isKeywordArgument()};
+  }
+
+  Optional<RenameAvailabilityInfo> info = renameAvailabilityInfo(VD, refInfo);
+  if (!info)
+    return None;
+
+  return RenameInfo{VD, *info};
 }
 
 class RenameRangeCollector : public IndexDataConsumer {
 public:
   RenameRangeCollector(StringRef USR, StringRef newName)
-      : USR(USR.str()), newName(newName.str()) {}
+      : USR(USR), newName(newName) {}
 
   RenameRangeCollector(const ValueDecl *D, StringRef newName)
-      : newName(newName.str()) {
-    llvm::raw_string_ostream OS(USR);
+      : newName(newName) {
+    SmallString<64> SS;
+    llvm::raw_svector_ostream OS(SS);
     printValueDeclUSR(D, OS);
+    USR = stringStorage.copyString(SS.str());
   }
 
+  RenameRangeCollector(RenameRangeCollector &&collector) = default;
+
   ArrayRef<RenameLoc> results() const { return locations; }
 
 private:
@@ -680,8 +705,8 @@ private:
                                              StringRef NewName);
 
 private:
-  std::string USR;
-  std::string newName;
+  StringRef USR;
+  StringRef newName;
   StringScratchSpace stringStorage;
   std::vector<RenameLoc> locations;
 };
@@ -836,29 +861,14 @@ class RefactoringAction##KIND: public RangeBasedRefactoringAction {           \
 
 bool RefactoringActionLocalRename::isApplicable(
     ResolvedCursorInfoPtr CursorInfo, DiagnosticEngine &Diag) {
-  auto ValueRefInfo = dyn_cast<ResolvedValueRefCursorInfo>(CursorInfo);
-  if (!ValueRefInfo)
-    return false;
-
-  Optional<RenameRefInfo> RefInfo;
-  if (ValueRefInfo->isRef())
-    RefInfo = {CursorInfo->getSourceFile(), CursorInfo->getLoc(),
-               ValueRefInfo->isKeywordArgument()};
-
-  auto RenameOp = getAvailableRenameForDecl(ValueRefInfo->getValueD(), RefInfo);
-  return RenameOp.has_value() &&
-    RenameOp.value() == RefactoringKind::LocalRename;
+  Optional<RenameInfo> Info = getRenameInfo(CursorInfo);
+  return Info &&
+         Info->Availability.AvailableKind == RenameAvailableKind::Available &&
+         Info->Availability.Kind == RefactoringKind::LocalRename;
 }
 
-static void analyzeRenameScope(ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
-                               DiagnosticEngine &Diags,
+static void analyzeRenameScope(ValueDecl *VD,
                                SmallVectorImpl<DeclContext *> &Scopes) {
-  Scopes.clear();
-  if (!getAvailableRenameForDecl(VD, RefInfo).has_value()) {
-    Diags.diagnose(SourceLoc(), diag::value_decl_no_loc, VD->getName());
-    return;
-  }
-
   auto *Scope = VD->getDeclContext();
   // There may be sibling decls that the renamed symbol is visible from.
   switch (Scope->getContextKind()) {
@@ -883,6 +893,53 @@ static void analyzeRenameScope(ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
   Scopes.push_back(Scope);
 }
 
+static Optional<RenameRangeCollector> localRenames(SourceFile *SF,
+                                                   SourceLoc startLoc,
+                                                   StringRef preferredName,
+                                                   DiagnosticEngine &diags) {
+  auto cursorInfo =
+      evaluateOrDefault(SF->getASTContext().evaluator,
+                        CursorInfoRequest{CursorInfoOwner(SF, startLoc)},
+                        new ResolvedCursorInfo());
+
+  Optional<RenameInfo> info = getRenameInfo(cursorInfo);
+  if (!info) {
+    diags.diagnose(startLoc, diag::unresolved_location);
+    return None;
+  }
+
+  switch (info->Availability.AvailableKind) {
+  case RenameAvailableKind::Available:
+    break;
+  case RenameAvailableKind::Unavailable_system_symbol:
+    diags.diagnose(startLoc, diag::decl_is_system_symbol, info->VD->getName());
+    return None;
+  case RenameAvailableKind::Unavailable_has_no_location:
+    diags.diagnose(startLoc, diag::value_decl_no_loc, info->VD->getName());
+    return None;
+  case RenameAvailableKind::Unavailable_has_no_name:
+    diags.diagnose(startLoc, diag::decl_has_no_name);
+    return None;
+  case RenameAvailableKind::Unavailable_has_no_accessibility:
+    diags.diagnose(startLoc, diag::decl_no_accessibility);
+    return None;
+  case RenameAvailableKind::Unavailable_decl_from_clang:
+    diags.diagnose(startLoc, diag::decl_from_clang);
+    return None;
+  }
+
+  SmallVector<DeclContext *, 8> scopes;
+  analyzeRenameScope(info->VD, scopes);
+  if (scopes.empty())
+    return None;
+
+  RenameRangeCollector rangeCollector(info->VD, preferredName);
+  for (DeclContext *DC : scopes)
+    indexDeclContext(DC, rangeCollector);
+
+  return rangeCollector;
+}
+
 bool RefactoringActionLocalRename::performChange() {
   if (StartLoc.isInvalid()) {
     DiagEngine.diagnose(SourceLoc(), diag::invalid_location);
@@ -897,40 +954,16 @@ bool RefactoringActionLocalRename::performChange() {
                         MD->getNameStr());
     return true;
   }
-  CursorInfo =
-      evaluateOrDefault(TheFile->getASTContext().evaluator,
-                        CursorInfoRequest{CursorInfoOwner(TheFile, StartLoc)},
-                        new ResolvedCursorInfo());
-  auto ValueRefCursorInfo = dyn_cast<ResolvedValueRefCursorInfo>(CursorInfo);
-  if (ValueRefCursorInfo && ValueRefCursorInfo->getValueD()) {
-    ValueDecl *VD = ValueRefCursorInfo->typeOrValue();
-    // The index always uses the outermost shadow for references
-    if (!ValueRefCursorInfo->getShorthandShadowedDecls().empty()) {
-      VD = ValueRefCursorInfo->getShorthandShadowedDecls().back();
-    }
 
-    SmallVector<DeclContext *, 8> Scopes;
-
-    Optional<RenameRefInfo> RefInfo;
-    if (ValueRefCursorInfo->isRef())
-      RefInfo = {CursorInfo->getSourceFile(), CursorInfo->getLoc(),
-                 ValueRefCursorInfo->isKeywordArgument()};
-
-    analyzeRenameScope(VD, RefInfo, DiagEngine, Scopes);
-    if (Scopes.empty())
+  Optional<RenameRangeCollector> rangeCollector =
+      localRenames(TheFile, StartLoc, PreferredName, DiagEngine);
+  if (!rangeCollector)
     return true;
-    RenameRangeCollector rangeCollector(VD, PreferredName);
-    for (DeclContext *DC : Scopes)
-      indexDeclContext(DC, rangeCollector);
 
   auto consumers = DiagEngine.takeConsumers();
   assert(consumers.size() == 1);
-    return syntacticRename(TheFile, rangeCollector.results(), EditConsumer,
+  return syntacticRename(TheFile, rangeCollector->results(), EditConsumer,
                          *consumers[0]);
-  } else {
-    DiagEngine.diagnose(StartLoc, diag::unresolved_location);
-    return true;
-  }
 }
 
 StringRef getDefaultPreferredName(RefactoringKind Kind) {
@@ -8783,9 +8816,9 @@ accept(SourceManager &SM, RegionType RegionType,
   }
 }
 
-void swift::ide::collectRenameAvailabilityInfo(
-    const ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
-    SmallVectorImpl<RenameAvailabilityInfo> &Infos) {
+Optional<RenameAvailabilityInfo>
+swift::ide::renameAvailabilityInfo(const ValueDecl *VD,
+                                   Optional<RenameRefInfo> RefInfo) {
   RenameAvailableKind AvailKind = RenameAvailableKind::Available;
   if (getRelatedSystemDecl(VD)){
     AvailKind = RenameAvailableKind::Unavailable_system_symbol;
@@ -8800,22 +8833,22 @@ void swift::ide::collectRenameAvailabilityInfo(
   if (isa<AbstractFunctionDecl>(VD)) {
     // Disallow renaming accessors.
     if (isa<AccessorDecl>(VD))
-      return;
+      return None;
 
     // Disallow renaming deinit.
     if (isa<DestructorDecl>(VD))
-      return;
+      return None;
 
     // Disallow renaming init with no arguments.
     if (auto CD = dyn_cast<ConstructorDecl>(VD)) {
       if (!CD->getParameters()->size())
-        return;
+        return None;
 
       if (RefInfo && !RefInfo->IsArgLabel) {
         NameMatcher Matcher(*(RefInfo->SF));
         auto Resolved = Matcher.resolve({RefInfo->Loc, /*ResolveArgs*/true});
         if (Resolved.LabelRanges.empty())
-          return;
+          return None;
       }
     }
 
@@ -8825,13 +8858,13 @@ void swift::ide::collectRenameAvailabilityInfo(
       // whether it's an instance method, so we do the same here for now.
       if (FD->getBaseIdentifier() == FD->getASTContext().Id_callAsFunction) {
         if (!FD->getParameters()->size())
-          return;
+          return None;
 
         if (RefInfo && !RefInfo->IsArgLabel) {
           NameMatcher Matcher(*(RefInfo->SF));
           auto Resolved = Matcher.resolve({RefInfo->Loc, /*ResolveArgs*/true});
           if (Resolved.LabelRanges.empty())
-            return;
+            return None;
         }
       }
     }
@@ -8839,16 +8872,13 @@ void swift::ide::collectRenameAvailabilityInfo(
 
   // Always return local rename for parameters.
   // FIXME: if the cursor is on the argument, we should return global rename.
-  if (isa<ParamDecl>(VD)) {
-    Infos.emplace_back(RefactoringKind::LocalRename, AvailKind);
-    return;
-  }
+  if (isa<ParamDecl>(VD))
+    return RenameAvailabilityInfo{RefactoringKind::LocalRename, AvailKind};
 
   // If the indexer considers VD a global symbol, then we apply global rename.
   if (index::isLocalSymbol(VD))
-    Infos.emplace_back(RefactoringKind::LocalRename, AvailKind);
-  else
-    Infos.emplace_back(RefactoringKind::GlobalRename, AvailKind);
+    return RenameAvailabilityInfo{RefactoringKind::LocalRename, AvailKind};
+  return RenameAvailabilityInfo{RefactoringKind::GlobalRename, AvailKind};
 }
 
 void swift::ide::collectAvailableRefactorings(
@@ -8858,26 +8888,9 @@ void swift::ide::collectAvailableRefactorings(
       CursorInfo->getSourceFile()->getASTContext().SourceMgr);
 
   if (!ExcludeRename) {
-    if (RefactoringActionLocalRename::isApplicable(CursorInfo, DiagEngine))
-      Kinds.push_back(RefactoringKind::LocalRename);
-
-    switch (CursorInfo->getKind()) {
-    case CursorInfoKind::ModuleRef:
-    case CursorInfoKind::Invalid:
-    case CursorInfoKind::StmtStart:
-    case CursorInfoKind::ExprStart:
-      break;
-    case CursorInfoKind::ValueRef: {
-      auto ValueRefInfo = cast<ResolvedValueRefCursorInfo>(CursorInfo);
-      Optional<RenameRefInfo> RefInfo;
-      if (ValueRefInfo->isRef())
-        RefInfo = {CursorInfo->getSourceFile(), CursorInfo->getLoc(),
-                   ValueRefInfo->isKeywordArgument()};
-      auto RenameOp =
-          getAvailableRenameForDecl(ValueRefInfo->getValueD(), RefInfo);
-      if (RenameOp.has_value() &&
-          RenameOp.value() == RefactoringKind::GlobalRename)
-        Kinds.push_back(RenameOp.value());
+    if (auto Info = getRenameInfo(CursorInfo)) {
+      if (Info->Availability.AvailableKind == RenameAvailableKind::Available) {
+        Kinds.push_back(Info->Availability.Kind);
       }
     }
   }
@@ -9084,29 +9097,11 @@ int swift::ide::findLocalRenameRanges(
   Diags.addConsumer(DiagConsumer);
 
   auto StartLoc = Lexer::getLocForStartOfToken(SM, Range.getStart(SM));
-  ResolvedCursorInfoPtr CursorInfo =
-      evaluateOrDefault(SF->getASTContext().evaluator,
-                        CursorInfoRequest{CursorInfoOwner(SF, StartLoc)},
-                        new ResolvedCursorInfo());
-  auto ValueRefCursorInfo = dyn_cast<ResolvedValueRefCursorInfo>(CursorInfo);
-  if (!ValueRefCursorInfo || !ValueRefCursorInfo->getValueD()) {
-    Diags.diagnose(StartLoc, diag::unresolved_location);
+  Optional<RenameRangeCollector> RangeCollector =
+      localRenames(SF, StartLoc, StringRef(), Diags);
+  if (!RangeCollector)
     return true;
-  }
-  ValueDecl *VD = ValueRefCursorInfo->typeOrValue();
-  Optional<RenameRefInfo> RefInfo;
-  if (ValueRefCursorInfo->isRef())
-    RefInfo = {CursorInfo->getSourceFile(), CursorInfo->getLoc(),
-               ValueRefCursorInfo->isKeywordArgument()};
 
-  llvm::SmallVector<DeclContext *, 8> Scopes;
-  analyzeRenameScope(VD, RefInfo, Diags, Scopes);
-  if (Scopes.empty())
-    return true;
-  RenameRangeCollector RangeCollector(VD, StringRef());
-  for (DeclContext *DC : Scopes)
-    indexDeclContext(DC, RangeCollector);
-
-  return findSyntacticRenameRanges(SF, RangeCollector.results(), RenameConsumer,
-                                   DiagConsumer);
+  return findSyntacticRenameRanges(SF, RangeCollector->results(),
+                                   RenameConsumer, DiagConsumer);
 }

test/SourceKit/Refactoring/shorthand_shadow.swift

@@ -0,0 +1,37 @@
+func renameInClosure() {
+  // RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+2):10 %s -- %s | %FileCheck %s --check-prefix=CLOSURE
+  // CLOSURE: [[# @LINE+1]]:10-[[# @LINE+1]]:18 source.refactoring.range.kind.basename
+  _ = { (toRename: Int?) in
+    // RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+2):12 %s -- %s | %FileCheck %s --check-prefix=CLOSURE
+    // CLOSURE: [[# @LINE+1]]:12-[[# @LINE+1]]:20 source.refactoring.range.kind.basename
+    if let toRename {
+      // RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+2):11 %s -- %s | %FileCheck %s --check-prefix=CLOSURE
+      // CLOSURE: [[# @LINE+1]]:11-[[# @LINE+1]]:19 source.refactoring.range.kind.basename
+      _ = toRename
+    }
+  }
+}
+
+func renameNonOptional() {
+  // RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+2):7 %s -- %s | %FileCheck %s --check-prefix=OPTIONAL
+  // OPTIONAL: [[# @LINE+1]]:7-[[# @LINE+1]]:18 source.refactoring.range.kind.basename
+  let nonOptional = 1
+  // RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+2):10 %s -- %s | %FileCheck %s --check-prefix=OPTIONAL
+  // OPTIONAL: [[# @LINE+1]]:10-[[# @LINE+1]]:21 source.refactoring.range.kind.basename
+  if let nonOptional {
+    // RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+2):9 %s -- %s | %FileCheck %s --check-prefix=OPTIONAL
+    // OPTIONAL: [[# @LINE+1]]:9-[[# @LINE+1]]:20 source.refactoring.range.kind.basename
+    _ = nonOptional
+  }
+}
+
+// Not ideal - we should allow renaming this locally. But it's an error case anyway.
+func renameBuiltinMacroWithoutHash() {
+  // RUN: not %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+1):10 %s -- %s 2>&1 | %FileCheck %s --check-prefix=BUILTIN
+  if let file {
+    // RUN: not %sourcekitd-test -req=find-local-rename-ranges -pos=%(line+1):9 %s -- %s 2>&1 | %FileCheck %s --check-prefix=BUILTIN
+    _ = file
+  }
+  // BUILTIN: error: cannot rename system symbol 'file'
+}
+

test/refactoring/RefactoringKind/member_init_class_as_value.swift

@@ -1,13 +0,0 @@
-class Person {
-  var firstName: String!
-  var lastName: String!
-  var age: Int!
-  var planet = "Earth", solarSystem = "Milky Way"
-  var avgHeight = 175
-}
-
-let _ = Person()
-
-// RUN: %refactor -source-filename %s -pos=9:10 | %FileCheck %s -check-prefix=CHECK-NONE
-// CHECK-NONE: Action begins
-// CHECK-NONE-NEXT: Action ends

test/refactoring/rename/shorthand_shadow.swift

@@ -1,16 +1,11 @@
-// Local rename starts from the `DeclContext` of the renamed `Decl`. For
-// closures that means we have no parents, so check that case specifically.
-
-func renameInClosure() {
-  // RUN: %refactor -rename -dump-text -source-filename %s -pos=%(line+2):10 -new-name=renamed | %FileCheck %s
-  // CHECK: shorthand_shadow.swift [[# @LINE+1]]:10 -> [[# @LINE+1]]:18
-  _ = { (toRename: Int?) in
-    // RUN: %refactor -rename -dump-text -source-filename %s -pos=%(line+2):12 -new-name=renamed | %FileCheck %s
-    // CHECK: shorthand_shadow.swift [[# @LINE+1]]:12 -> [[# @LINE+1]]:20
-    if let toRename {
-      // RUN: %refactor -rename -dump-text -source-filename %s -pos=%(line+2):11 -new-name=renamed | %FileCheck %s
-      // CHECK: shorthand_shadow.swift [[# @LINE+1]]:11 -> [[# @LINE+1]]:19
-      _ = toRename
-    }
+// RUN: %refactor -rename -dump-text -source-filename %s -pos=%(line+2):29 -new-name=renamed | %FileCheck %s --check-prefix=OPTIONAL
+// OPTIONAL: shorthand_shadow.swift [[# @LINE+1]]:29 -> [[# @LINE+1]]:32
+func renameShorthandBinding(opt: Int?) {
+  // RUN: %refactor -rename -dump-text -source-filename %s -pos=%(line+2):10 -new-name=renamed | %FileCheck %s --check-prefix=OPTIONAL
+  // OPTIONAL: shorthand_shadow.swift [[# @LINE+1]]:10 -> [[# @LINE+1]]:13
+  if let opt {
+    // RUN: %refactor -rename -dump-text -source-filename %s -pos=%(line+2):9 -new-name=renamed | %FileCheck %s --check-prefix=OPTIONAL
+    // OPTIONAL: shorthand_shadow.swift [[# @LINE+1]]:9 -> [[# @LINE+1]]:12
+    _ = opt
   }
 }

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

@@ -704,14 +704,14 @@ static bool passCursorInfoForModule(ModuleEntity Mod,
 static void
 collectAvailableRenameInfo(const ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
                            SmallVectorImpl<RefactoringInfo> &Refactorings) {
-  SmallVector<RenameAvailabilityInfo, 2> Renames;
-  collectRenameAvailabilityInfo(VD, RefInfo, Renames);
-  for (auto Info : Renames) {
+  Optional<RenameAvailabilityInfo> Info = renameAvailabilityInfo(VD, RefInfo);
+  if (!Info)
+    return;
+
   Refactorings.emplace_back(
-        SwiftLangSupport::getUIDForRefactoringKind(Info.Kind),
-        ide::getDescriptiveRefactoringKindName(Info.Kind),
-        ide::getDescriptiveRenameUnavailableReason(Info.AvailableKind));
-  }
+      SwiftLangSupport::getUIDForRefactoringKind(Info->Kind),
+      ide::getDescriptiveRefactoringKindName(Info->Kind),
+      ide::getDescriptiveRenameUnavailableReason(Info->AvailableKind));
 }
 
 static void collectAvailableRefactoringsOtherThanRename(