Replace uses of presumed locations where they do not make sense

bnbarham · bnbarham · commit 20f45ec284a7 · 2021-04-10T09:49:31.000+10:00
Various uses of `getPresumedLineAndColumnForLoc` were likely added when
that function was the very misleading name `getLineAndColumn`. Change
these to use `getLineAndColumnForBuffer` instead where appropriate, ie.
we want the underlying file rather than the location to display to the
user.

There were also some cases where the buffer identifier had been swapped
to use the display name instead, under the assumption that the presumed
location was needed. Updated those as well.

SingleRawComment: Lines are only used when merging comments, where the
original location is fine to use.

Index: Doesn't store the file set in #sourceLocation, so using the
presumed line would end up pointing to a location that makes no sense.

Editor functionality: Formatting and refactoring are on the current
file. Using the presumed location would result in incorrect
replacements.
diff --git a/lib/AST/RawComment.cpp b/lib/AST/RawComment.cpp
@@ -62,7 +62,7 @@ SingleRawComment::SingleRawComment(CharSourceRange Range,
     : Range(Range), RawText(SourceMgr.extractText(Range)),
       Kind(static_cast<unsigned>(getCommentKind(RawText))) {
   auto StartLineAndColumn =
-      SourceMgr.getPresumedLineAndColumnForLoc(Range.getStart());
+      SourceMgr.getLineAndColumnInBuffer(Range.getStart());
   StartLine = StartLineAndColumn.first;
   StartColumn = StartLineAndColumn.second;
   EndLine = SourceMgr.getLineAndColumnInBuffer(Range.getEnd()).first;
diff --git a/lib/IDE/Formatting.cpp b/lib/IDE/Formatting.cpp
@@ -333,7 +333,7 @@ class FormatContext {
 
   std::pair<unsigned, unsigned> indentLineAndColumn() {
     if (InnermostCtx)
-      return SM.getPresumedLineAndColumnForLoc(InnermostCtx->ContextLoc);
+      return SM.getLineAndColumnInBuffer(InnermostCtx->ContextLoc);
     return std::make_pair(0, 0);
   }
 
diff --git a/lib/IDE/IDERequests.cpp b/lib/IDE/IDERequests.cpp
@@ -340,7 +340,7 @@ void swift::simple_display(llvm::raw_ostream &out, const CursorInfoOwner &owner)
     return;
   auto &SM = owner.File->getASTContext().SourceMgr;
   out << SM.getIdentifierForBuffer(*owner.File->getBufferID());
-  auto LC = SM.getPresumedLineAndColumnForLoc(owner.Loc);
+  auto LC = SM.getLineAndColumnInBuffer(owner.Loc);
   out << ":" << LC.first << ":" << LC.second;
 }
 
@@ -351,7 +351,7 @@ void swift::ide::simple_display(llvm::raw_ostream &out,
   out << "Resolved cursor info at ";
   auto &SM = info.SF->getASTContext().SourceMgr;
   out << SM.getIdentifierForBuffer(*info.SF->getBufferID());
-  auto LC = SM.getPresumedLineAndColumnForLoc(info.Loc);
+  auto LC = SM.getLineAndColumnInBuffer(info.Loc);
   out << ":" << LC.first << ":" << LC.second;
 }
 
@@ -1072,8 +1072,8 @@ void swift::simple_display(llvm::raw_ostream &out,
     return;
   auto &SM = owner.File->getASTContext().SourceMgr;
   out << SM.getIdentifierForBuffer(*owner.File->getBufferID());
-  auto SLC = SM.getPresumedLineAndColumnForLoc(owner.StartLoc);
-  auto ELC = SM.getPresumedLineAndColumnForLoc(owner.EndLoc);
+  auto SLC = SM.getLineAndColumnInBuffer(owner.StartLoc);
+  auto ELC = SM.getLineAndColumnInBuffer(owner.EndLoc);
   out << ": (" << SLC.first << ":" << SLC.second << ", "
     << ELC.first << ":" << ELC.second << ")";
 }
diff --git a/lib/IDE/Refactoring.cpp b/lib/IDE/Refactoring.cpp
@@ -1189,7 +1189,7 @@ getNotableRegions(StringRef SourceText, unsigned NameOffset, StringRef Name,
   unsigned BufferId = Instance->getPrimarySourceFile()->getBufferID().getValue();
   SourceManager &SM = Instance->getSourceMgr();
   SourceLoc NameLoc = SM.getLocForOffset(BufferId, NameOffset);
-  auto LineAndCol = SM.getPresumedLineAndColumnForLoc(NameLoc);
+  auto LineAndCol = SM.getLineAndColumnInBuffer(NameLoc);
 
   UnresolvedLoc UnresoledName{NameLoc, true};
 
@@ -1210,8 +1210,8 @@ getNotableRegions(StringRef SourceText, unsigned NameOffset, StringRef Name,
   llvm::transform(
       Ranges, NoteRegions.begin(),
       [&SM](RenameRangeDetail &Detail) -> NoteRegion {
-        auto Start = SM.getPresumedLineAndColumnForLoc(Detail.Range.getStart());
-        auto End = SM.getPresumedLineAndColumnForLoc(Detail.Range.getEnd());
+        auto Start = SM.getLineAndColumnInBuffer(Detail.Range.getStart());
+        auto End = SM.getLineAndColumnInBuffer(Detail.Range.getEnd());
         return {Detail.RangeKind, Start.first, Start.second,
                 End.first,        End.second,  Detail.Index};
       });
diff --git a/lib/Index/Index.cpp b/lib/Index/Index.cpp
@@ -603,7 +603,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
   getLineColAndOffset(SourceLoc Loc) {
     if (Loc.isInvalid())
       return std::make_tuple(0, 0, None);
-    auto lineAndColumn = SrcMgr.getPresumedLineAndColumnForLoc(Loc, BufferID);
+    auto lineAndColumn = SrcMgr.getLineAndColumnInBuffer(Loc, BufferID);
     unsigned offset = SrcMgr.getLocOffsetInBuffer(Loc, BufferID);
     return std::make_tuple(lineAndColumn.first, lineAndColumn.second, offset);
   }
diff --git a/test/Index/index_location_directive.swift b/test/Index/index_location_directive.swift
@@ -0,0 +1,10 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s | %FileCheck %s
+
+// The index should probably use the presumed location, but for now just check
+// that the indexed location isn't a mixture of both.
+
+#sourceLocation(file: "some_file.swift", line: 1)
+func testFunc() {}
+// CHECK: [[@LINE-1]]:6 | function/Swift | testFunc() | s:14swift_ide_test0C4FuncyyF | Def | rel: 0
+#sourceLocation()
diff --git a/test/SourceKit/CodeFormat/indent-location-directive.swift b/test/SourceKit/CodeFormat/indent-location-directive.swift
@@ -0,0 +1,8 @@
+#sourceLocation(file: "someFile.swift", line: 10)
+
+    func foo() {
+// RUN: %sourcekitd-test -req=format -pos=%(line+1):1 %s | %FileCheck %s
+let test = 1
+// CHECK: key.sourcetext: "        let test = 1"
+    }
+#sourceLocation()
diff --git a/test/SourceKit/Refactoring/rename-with-location-directive.swift b/test/SourceKit/Refactoring/rename-with-location-directive.swift
@@ -0,0 +1,6 @@
+#sourceLocation(file: "someFile.swift", line: 10)
+func test() {}
+#sourceLocation()
+
+// RUN: %sourcekitd-test -req=find-local-rename-ranges -pos=2:6 %s -- %s | %FileCheck %s
+// CHECK: 2:6-2:10 source.refactoring.range.kind.basename
diff --git a/test/SourceKit/Refactoring/semantic-refactoring/localize-string.swift b/test/SourceKit/Refactoring/semantic-refactoring/localize-string.swift
@@ -1,9 +1,18 @@
+// REQUIRES: OS=macosx || OS=linux-gnu
+
 func foo() -> String {
+  // RUN: %sourcekitd-test -req=localize-string -pos=%(line+1):10 %s -- %s | %FileCheck %s --check-prefix=CHECK-BASIC
   return "abc"
+  // CHECK-BASIC: source.edit.kind.active:
+  // CHECK-BASIC: [[# @LINE-2]]:10-[[# @LINE-2]]:10 "NSLocalizedString("
+  // CHECK-BASIC: source.edit.kind.active:
+  // CHECK-BASIC: [[# @LINE-4]]:15-[[# @LINE-4]]:15 ", comment: "")"
 }
 
-// RUN: %empty-directory(%t.result)
-// RUN: %sourcekitd-test -req=localize-string -pos=2:10 %s -- %s > %t.result/localize-string.swift.expected
-// RUN: %diff -u %S/localize-string.swift.expected %t.result/localize-string.swift.expected
-
-// REQUIRES: OS=macosx || OS=linux-gnu
+#sourceLocation(file: "someFile.swift", line: 20)
+func bar() -> String {
+  // RUN: %sourcekitd-test -req=localize-string -pos=%(line+1):10 %s -- %s | %FileCheck %s --check-prefix=CHECK-DIRECTIVE
+  return "abc"
+  // CHECK-DIRECTIVE: [[# @LINE-1]]:10-[[# @LINE-1]]:10
+}
+#sourceLocation()
diff --git a/test/SourceKit/Refactoring/semantic-refactoring/localize-string.swift.expected b/test/SourceKit/Refactoring/semantic-refactoring/localize-string.swift.expected
diff --git a/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp b/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp
@@ -1233,11 +1233,8 @@ class RequestRefactoringEditConsumer::Implementation {
     llvm::transform(
         Replacements, std::back_inserter(AllEdits),
         [&](const Replacement &R) -> Edit {
-          std::pair<unsigned, unsigned> Start =
-                                            SM.getPresumedLineAndColumnForLoc(
-                                                R.Range.getStart()),
-                                        End = SM.getPresumedLineAndColumnForLoc(
-                                            R.Range.getEnd());
+          auto Start = SM.getLineAndColumnInBuffer(R.Range.getStart());
+          auto End = SM.getLineAndColumnInBuffer(R.Range.getEnd());
           SmallVector<NoteRegion, 4> SubRanges;
           auto RawRanges = R.RegionsWorthNote;
           llvm::transform(
@@ -1305,9 +1302,9 @@ class RequestRenameRangeConsumer::Implementation {
     for (const auto &R : Ranges) {
       SourceKit::RenameRangeDetail Result;
       std::tie(Result.StartLine, Result.StartColumn) =
-          SM.getPresumedLineAndColumnForLoc(R.Range.getStart());
+          SM.getLineAndColumnInBuffer(R.Range.getStart());
       std::tie(Result.EndLine, Result.EndColumn) =
-          SM.getPresumedLineAndColumnForLoc(R.Range.getEnd());
+          SM.getLineAndColumnInBuffer(R.Range.getEnd());
       Result.ArgIndex = R.Index;
       Result.Kind =
           SwiftLangSupport::getUIDForRefactoringRangeKind(R.RangeKind);
diff --git a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp
@@ -1312,7 +1312,7 @@ static void resolveCursor(
         SmallVector<RefactoringKind, 8> Kinds;
         RangeConfig Range;
         Range.BufferId = BufferID;
-        auto Pair = SM.getPresumedLineAndColumnForLoc(Loc);
+        auto Pair = SM.getLineAndColumnInBuffer(Loc);
         Range.Line = Pair.first;
         Range.Column = Pair.second;
         Range.Length = Length;
diff --git a/tools/swift-syntax-test/swift-syntax-test.cpp b/tools/swift-syntax-test/swift-syntax-test.cpp
@@ -702,7 +702,7 @@ int doDumpRawTokenSyntax(const StringRef InputFile) {
     SourceLoc Loc =
         SourceMgr.getLocForOffset(BufferID, TokAndPos.second.getOffset());
     unsigned Line, Column;
-    std::tie(Line, Column) = SourceMgr.getPresumedLineAndColumnForLoc(Loc);
+    std::tie(Line, Column) = SourceMgr.getLineAndColumnInBuffer(Loc);
     llvm::outs() << Line << ":" << Column << "\n";
     TokAndPos.first->dump(llvm::outs());
     llvm::outs() << "\n";

Original file line number	Diff line number	Diff line change
`@@ -333,7 +333,7 @@ class FormatContext {`
`333`	`333`
`334`	`334`	`std::pair<unsigned, unsigned> indentLineAndColumn() {`
`335`	`335`	`if (InnermostCtx)`
`336`		`- return SM.getPresumedLineAndColumnForLoc(InnermostCtx->ContextLoc);`
	`336`	`+ return SM.getLineAndColumnInBuffer(InnermostCtx->ContextLoc);`
`337`	`337`	`return std::make_pair(0, 0);`
`338`	`338`	`}`
`339`	`339`
Original file line number	Diff line number	Diff line change
`@@ -603,7 +603,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {`
`603`	`603`	`getLineColAndOffset(SourceLoc Loc) {`
`604`	`604`	`if (Loc.isInvalid())`
`605`	`605`	`return std::make_tuple(0, 0, None);`
`606`		`- auto lineAndColumn = SrcMgr.getPresumedLineAndColumnForLoc(Loc, BufferID);`
	`606`	`+ auto lineAndColumn = SrcMgr.getLineAndColumnInBuffer(Loc, BufferID);`
`607`	`607`	`unsigned offset = SrcMgr.getLocOffsetInBuffer(Loc, BufferID);`
`608`	`608`	`return std::make_tuple(lineAndColumn.first, lineAndColumn.second, offset);`
`609`	`609`	`}`