[SourceKit] Merge RenameLoc and RenameLocation

It’s easier to understand the code if we don’t have these two nearly, but not quite, identical types.

Author: ahoppen

include/swift/Refactoring/Refactoring.h

@@ -10,14 +10,15 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef SWIFT_IDE_REFACTORING_H
-#define SWIFT_IDE_REFACTORING_H
+#ifndef SWIFT_REFACTORING_REFACTORING_H
+#define SWIFT_REFACTORING_REFACTORING_H
 
 #include "swift/AST/DiagnosticConsumer.h"
 #include "swift/Basic/LLVM.h"
 #include "swift/Basic/StringExtras.h"
 #include "swift/IDE/CancellableResult.h"
 #include "swift/IDE/Utils.h"
+#include "swift/Refactoring/RenameLoc.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace swift {
@@ -61,21 +62,6 @@ struct RenameInfo {
 
 llvm::Optional<RenameInfo> getRenameInfo(ResolvedCursorInfoPtr cursorInfo);
 
-enum class NameUsage {
-  Unknown,
-  Reference,
-  Definition,
-  Call
-};
-
-struct RenameLoc {
-  unsigned Line;
-  unsigned Column;
-  NameUsage Usage;
-  StringRef OldName;
-  const bool IsFunctionLike;
-};
-
 /// An array of \c RenameLoc that also keeps the underlying string storage of
 /// the \c StringRef inside the \c RenameLoc alive.
 class RenameLocs {
@@ -184,4 +170,4 @@ collectRefactorings(ResolvedCursorInfoPtr CursorInfo, bool ExcludeRename);
 } // namespace ide
 } // namespace swift
 
-#endif // SWIFT_IDE_REFACTORING_H
+#endif // SWIFT_REFACTORING_REFACTORING_H

include/swift/Refactoring/RenameLoc.h

@@ -0,0 +1,86 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_REFACTORING_RENAMELOC_H
+#define SWIFT_REFACTORING_RENAMELOC_H
+
+#include "swift/Basic/LLVM.h"
+
+namespace swift {
+namespace ide {
+
+/// Describes how a `ResolvedLoc` is being used
+enum class RenameLocUsage {
+  /// The definition of a function/subscript/variable/...
+  Definition,
+
+  /// The symbol is being referenced.
+  ///
+  /// This includes
+  ///  - References to variables
+  ///  - Unapplied references to functions (`myStruct.memberFunc`)
+  ///  - Calls to subscripts (`myArray[1]`, location is `[` here, length 1)
+  Reference,
+
+  /// A function that is being called.
+  Call,
+
+  /// Unknown name usage occurs if we don't have an entry in the index that
+  /// tells us whether the location is a call, reference or a definition. The
+  /// most common reasons why this happens is if the editor is adding syntactic
+  /// results (eg. from comments or string literals).
+  Unknown
+};
+
+/// The input to `findSyntacticRenameRanges`.
+///
+/// Specifies the location of a base name for which `findSyntacticRenameRanges`
+/// should find the argument labels as well as some semantic information needed
+/// to resolve the rename ranges.
+struct RenameLoc {
+  /// The line at which the base name starts (1-based).
+  unsigned Line;
+
+  /// The column at which the base name (excluding trivia) starts (1-based).
+  unsigned Column;
+
+  ///  /// The offset at which the related name starts.
+  unsigned Offset;
+
+  /// The length of the base name in the related identifier. For functions,
+  /// this does not include the parameters/arguments.
+  unsigned Length;
+
+  /// How the identifier is being used (call, reference, definition, unknown).
+  ///
+  /// Used to decide whether a given occurance should be renamed and/or if its
+  /// argument labels should be renamed.
+  RenameLocUsage Usage;
+
+  /// The old decl name being renamed.
+  ///
+  /// ### Examples
+  /// - `foo(a:b:)` for a function with two parameters.
+  /// - `foo` for a variable.
+  /// - `foo(_:)` for a function with a single unnamed parameter
+  /// - `foo()` for a function without parameters.
+  StringRef OldName;
+
+  RenameLoc(unsigned Line, unsigned Column, RenameLocUsage Usage,
+            StringRef OldName)
+      : Line(Line), Column(Column), Usage(Usage), OldName(OldName) {}
+};
+
+} // namespace ide
+} // namespace swift
+
+#endif // SWIFT_REFACTORING_RENAMELOC_H

lib/Refactoring/ExtractFunction.cpp

@@ -77,10 +77,8 @@ static SourceLoc getNewFuncInsertLoc(DeclContext *DC,
   return SourceLoc();
 }
 
-static std::vector<NoteRegion> getNotableRegions(StringRef SourceText,
-                                                 unsigned NameOffset,
-                                                 StringRef Name,
-                                                 bool IsFunctionLike = false) {
+static std::vector<NoteRegion>
+getNotableRegions(StringRef SourceText, unsigned NameOffset, StringRef Name) {
   auto InputBuffer =
       llvm::MemoryBuffer::getMemBufferCopy(SourceText, "<extract>");
 
@@ -106,8 +104,7 @@ static std::vector<NoteRegion> getNotableRegions(StringRef SourceText,
   assert(!Resolved.empty() && "Failed to resolve generated func name loc");
 
   RenameLoc RenameConfig = {LineAndCol.first, LineAndCol.second,
-                            NameUsage::Definition, /*OldName=*/Name,
-                            IsFunctionLike};
+                            RenameLocUsage::Definition, /*OldName=*/Name};
   std::vector<RenameRangeDetail> Ranges =
       getSyntacticRenameRangeDetails(SM, Name, Resolved.back(), RenameConfig)
           .Ranges;
@@ -290,13 +287,11 @@ bool RefactoringActionExtractFunction::performChange() {
 
   StringRef DeclStr(Buffer.begin() + FuncBegin, FuncEnd - FuncBegin);
   auto NotableFuncRegions =
-      getNotableRegions(DeclStr, FuncNameOffset, ExtractedFuncName,
-                        /*IsFunctionLike=*/true);
+      getNotableRegions(DeclStr, FuncNameOffset, ExtractedFuncName);
 
   StringRef CallStr(Buffer.begin() + ReplaceBegin, ReplaceEnd - ReplaceBegin);
   auto NotableCallRegions =
-      getNotableRegions(CallStr, CallNameOffset, ExtractedFuncName,
-                        /*IsFunctionLike=*/true);
+      getNotableRegions(CallStr, CallNameOffset, ExtractedFuncName);
 
   // Insert the new function's declaration.
   EditConsumer.accept(SM, InsertLoc, DeclStr, NotableFuncRegions);

lib/Refactoring/LocalRename.cpp

@@ -219,7 +219,6 @@ class RenameRangeCollector : public IndexDataConsumer {
           locations.push_back(std::move(*loc));
         } else {
           assert(existingLoc->OldName == loc->OldName &&
-                 existingLoc->IsFunctionLike == loc->IsFunctionLike &&
                  "Asked to do a different rename for the same location?");
         }
       }
@@ -241,37 +240,19 @@ RenameRangeCollector::indexSymbolToRenameLoc(const index::IndexSymbol &symbol) {
     return llvm::None;
   }
 
-  NameUsage usage = NameUsage::Unknown;
+  RenameLocUsage usage = RenameLocUsage::Unknown;
   if (symbol.roles & (unsigned)index::SymbolRole::Call) {
-    usage = NameUsage::Call;
+    usage = RenameLocUsage::Call;
   } else if (symbol.roles & (unsigned)index::SymbolRole::Definition) {
-    usage = NameUsage::Definition;
+    usage = RenameLocUsage::Definition;
   } else if (symbol.roles & (unsigned)index::SymbolRole::Reference) {
-    usage = NameUsage::Reference;
+    usage = RenameLocUsage::Reference;
   } else {
     llvm_unreachable("unexpected role");
   }
 
-  bool isFunctionLike = false;
-
-  switch (symbol.symInfo.Kind) {
-  case index::SymbolKind::EnumConstant:
-  case index::SymbolKind::Function:
-  case index::SymbolKind::Constructor:
-  case index::SymbolKind::ConversionFunction:
-  case index::SymbolKind::InstanceMethod:
-  case index::SymbolKind::ClassMethod:
-  case index::SymbolKind::StaticMethod:
-    isFunctionLike = true;
-    break;
-  case index::SymbolKind::Class:
-  case index::SymbolKind::Enum:
-  case index::SymbolKind::Struct:
-  default:
-    break;
-  }
   StringRef oldName = stringStorage->copyString(symbol.name);
-  return RenameLoc{symbol.line, symbol.column, usage, oldName, isFunctionLike};
+  return RenameLoc{symbol.line, symbol.column, usage, oldName};
 }
 
 /// Get the decl context that we need to walk when renaming \p VD.

lib/Refactoring/SyntacticRename.cpp

@@ -63,7 +63,7 @@ swift::ide::resolveRenameLocations(ArrayRef<RenameLoc> RenameLocs,
         return {};
       }
 
-      if (RenameLoc.Usage == NameUsage::Call && !RenameLoc.IsFunctionLike) {
+      if (RenameLoc.Usage == RenameLocUsage::Call && !OldName.isFunction()) {
         Diags.diagnose(Location, diag::name_not_functionlike, NewName);
         return {};
       }

lib/Refactoring/SyntacticRenameRangeDetails.cpp

@@ -396,13 +396,13 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
   if (!resolved.range.isValid())
     return RegionType::Unmatched;
 
-  NameUsage usage = config.Usage;
+  RenameLocUsage usage = config.Usage;
 
   auto regionKind = getSyntacticRenameRegionType(resolved);
 
   SpecialBaseName specialBaseName = specialBaseNameFor(Old);
 
-  if (usage == NameUsage::Unknown) {
+  if (usage == RenameLocUsage::Unknown) {
     // Unknown name usage occurs if we don't have an entry in the index that
     // tells us whether the location is a call, reference or a definition. The
     // most common reasons why this happens is if the editor is adding syntactic
@@ -439,7 +439,8 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
       // be called as `myStruct()`, so even if the base fails to be renamed,
       // continue.
       // But the names do need to match for definitions and references.
-      if (usage == NameUsage::Definition || usage == NameUsage::Reference) {
+      if (usage == RenameLocUsage::Definition ||
+          usage == RenameLocUsage::Reference) {
         return RegionType::Mismatch;
       }
     }
@@ -449,7 +450,7 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
     // Accesses to the subscript are modelled as references with `[` as the
     // base name, which does not match. Subscripts are never called in the
     // index.
-    if (usage == NameUsage::Definition) {
+    if (usage == RenameLocUsage::Definition) {
       if (renameBase(resolved.range, RefactoringRangeKind::KeywordBaseName)) {
         return RegionType::Mismatch;
       }
@@ -461,18 +462,18 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
   bool isCallSite = false;
   if (Old.isFunction()) {
     switch (usage) {
-    case NameUsage::Call:
+    case RenameLocUsage::Call:
       // All calls except for operators have argument labels that should be
       // renamed.
       handleLabels = !Lexer::isOperator(Old.base());
       isCallSite = true;
       break;
-    case NameUsage::Definition:
+    case RenameLocUsage::Definition:
       // All function definitions have argument labels that should be renamed.
       handleLabels = true;
       isCallSite = false;
       break;
-    case NameUsage::Reference:
+    case RenameLocUsage::Reference:
       if (resolved.labelType == LabelRangeType::CompoundName) {
         // If we have a compound name that specifies argument labels to
         // disambiguate functions with the same base name, we always need to
@@ -490,7 +491,7 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
         isCallSite = false;
       }
       break;
-    case NameUsage::Unknown:
+    case RenameLocUsage::Unknown:
       // If we don't know where the function is used, fall back to trying to
       // rename labels if there are some.
       handleLabels = resolved.labelType != LabelRangeType::None;
@@ -504,8 +505,8 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
   }
   if (renameLabels(resolved.labelRanges, resolved.firstTrailingLabel,
                    resolved.labelType, isCallSite)) {
-    return usage == NameUsage::Unknown ? RegionType::Unmatched
-                                       : RegionType::Mismatch;
+    return usage == RenameLocUsage::Unknown ? RegionType::Unmatched
+                                            : RegionType::Mismatch;
   }
 
   return regionKind;

tools/SourceKit/include/SourceKit/Core/LangSupport.h

@@ -19,6 +19,7 @@
 #include "swift/AST/Type.h"
 #include "swift/IDE/CancellableResult.h"
 #include "swift/IDE/CodeCompletionResult.h"
+#include "swift/Refactoring/RenameLoc.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
@@ -37,6 +38,7 @@ namespace llvm {
 namespace SourceKit {
 class GlobalConfig;
 using swift::ide::CancellableResult;
+using swift::ide::RenameLoc;
 
 struct EntityInfo {
   UIdent Kind;
@@ -893,25 +895,6 @@ struct CategorizedRenameRanges {
   std::vector<RenameRangeDetail> Ranges;
 };
 
-enum class RenameType {
-  Unknown,
-  Definition,
-  Reference,
-  Call
-};
-
-struct RenameLocation {
-  unsigned Line;
-  unsigned Column;
-  RenameType Type;
-};
-
-struct RenameLocations {
-  StringRef OldName;
-  const bool IsFunctionLike;
-  std::vector<RenameLocation> LineColumnLocs;
-};
-
 struct IndexStoreOptions {
   std::string IndexStorePath;
   std::string IndexUnitOutputPath;
@@ -1192,7 +1175,7 @@ class LangSupport {
 
   virtual CancellableResult<std::vector<CategorizedRenameRanges>>
   findRenameRanges(llvm::MemoryBuffer *InputBuf,
-                   ArrayRef<RenameLocations> RenameLocations,
+                   ArrayRef<RenameLoc> RenameLocations,
                    ArrayRef<const char *> Args) = 0;
   virtual void
   findLocalRenameRanges(StringRef Filename, unsigned Line, unsigned Column,