[incrParse] Fix bug mapping a node's location back to its location in the cached syntax tree

include/swift/Parse/SyntaxParsingCache.h

@@ -18,7 +18,9 @@
 #include "llvm/Support/raw_ostream.h"
 #include <unordered_set>
 
-namespace {
+namespace swift {
+
+using namespace swift::syntax;
 
 /// A single edit to the original source file in which a continuous range of
 /// characters have been replaced by a new string
@@ -38,16 +40,10 @@ struct SourceEdit {
   /// Check if the characters replaced by this edit fall into the given range
   /// or are directly adjacent to it
   bool intersectsOrTouchesRange(size_t RangeStart, size_t RangeEnd) {
-    return !(End <= RangeStart || Start >= RangeEnd);
+    return End >= RangeStart && Start <= RangeEnd;
   }
 };
 
-} // anonymous namespace
-
-namespace swift {
-
-using namespace swift::syntax;
-
 struct SyntaxReuseRegion {
   AbsolutePosition Start;
   AbsolutePosition End;
@@ -89,6 +85,13 @@ class SyntaxParsingCache {
   std::vector<SyntaxReuseRegion>
   getReusedRegions(const SourceFileSyntax &SyntaxTree) const;
 
+  /// Translates a post-edit position to a pre-edit position by undoing the
+  /// specified edits.
+  /// Should not be invoked externally. Only public for testing purposes.
+  static size_t
+  translateToPreEditPosition(size_t PostEditPosition,
+                             llvm::SmallVector<SourceEdit, 4> Edits);
+
 private:
   llvm::Optional<Syntax> lookUpFrom(const Syntax &Node, size_t NodeStart,
                                     size_t Position, SyntaxKind Kind);

lib/Basic/SourceLoc.cpp

@@ -331,7 +331,9 @@ llvm::Optional<unsigned> SourceManager::resolveFromLineCol(unsigned BufferId,
     return None;
   }
   Ptr = LineStart;
-  for (; Ptr < End; ++Ptr) {
+
+  // The <= here is to allow for non-inclusive range end positions at EOF
+  for (; Ptr <= End; ++Ptr) {
     --Col;
     if (Col == 0)
       return Ptr - InputBuf->getBufferStart();

lib/Parse/SyntaxParsingCache.cpp

@@ -79,17 +79,21 @@ llvm::Optional<Syntax> SyntaxParsingCache::lookUpFrom(const Syntax &Node,
   return llvm::None;
 }
 
-llvm::Optional<Syntax> SyntaxParsingCache::lookUp(size_t NewPosition,
-                                                  SyntaxKind Kind) {
-  // Undo the edits in reverse order
-  size_t OldPosition = NewPosition;
-  for (auto I = Edits.rbegin(), E = Edits.rend(); I != E; ++I) {
+size_t SyntaxParsingCache::translateToPreEditPosition(
+    size_t PostEditPosition, llvm::SmallVector<SourceEdit, 4> Edits) {
+  size_t Position = PostEditPosition;
+  for (auto I = Edits.begin(), E = Edits.end(); I != E; ++I) {
     auto Edit = *I;
-    if (Edit.End <= OldPosition) {
-      OldPosition =
-          OldPosition - Edit.ReplacementLength + Edit.originalLength();
+    if (Edit.End + Edit.ReplacementLength - Edit.originalLength() <= Position) {
+      Position = Position - Edit.ReplacementLength + Edit.originalLength();
     }
   }
+  return Position;
+}
+
+llvm::Optional<Syntax> SyntaxParsingCache::lookUp(size_t NewPosition,
+                                                  SyntaxKind Kind) {
+  size_t OldPosition = translateToPreEditPosition(NewPosition, Edits);
 
   auto Node = lookUpFrom(OldSyntaxTree, /*NodeStart=*/0, OldPosition, Kind);
   if (Node.hasValue()) {

test/incrParse/Outputs/extend-identifier-at-eof.json

@@ -0,0 +1,132 @@
+{
+  "id": 39,
+  "kind": "SourceFile",
+  "layout": [
+    {
+      "id": 38,
+      "kind": "CodeBlockItemList",
+      "layout": [
+        {
+          "id": 13,
+          "omitted": true
+        },
+        {
+          "id": 35,
+          "kind": "CodeBlockItem",
+          "layout": [
+            {
+              "id": 34,
+              "kind": "SequenceExpr",
+              "layout": [
+                {
+                  "id": 33,
+                  "kind": "ExprList",
+                  "layout": [
+                    {
+                      "id": 28,
+                      "kind": "DiscardAssignmentExpr",
+                      "layout": [
+                        {
+                          "id": 27,
+                          "tokenKind": {
+                            "kind": "kw__"
+                          },
+                          "leadingTrivia": [
+                            {
+                              "kind": "Newline",
+                              "value": 1
+                            },
+                            {
+                              "kind": "LineComment",
+                              "value": "\/\/ ATTENTION: This file is testing the EOF token. "
+                            },
+                            {
+                              "kind": "Newline",
+                              "value": 1
+                            },
+                            {
+                              "kind": "LineComment",
+                              "value": "\/\/ DO NOT PUT ANYTHING AFTER THE CHANGE, NOT EVEN A NEWLINE"
+                            },
+                            {
+                              "kind": "Newline",
+                              "value": 1
+                            }
+                          ],
+                          "trailingTrivia": [
+                            {
+                              "kind": "Space",
+                              "value": 1
+                            }
+                          ],
+                          "presence": "Present"
+                        }
+                      ],
+                      "presence": "Present"
+                    },
+                    {
+                      "id": 30,
+                      "kind": "AssignmentExpr",
+                      "layout": [
+                        {
+                          "id": 29,
+                          "tokenKind": {
+                            "kind": "equal"
+                          },
+                          "leadingTrivia": [],
+                          "trailingTrivia": [
+                            {
+                              "kind": "Space",
+                              "value": 1
+                            }
+                          ],
+                          "presence": "Present"
+                        }
+                      ],
+                      "presence": "Present"
+                    },
+                    {
+                      "id": 32,
+                      "kind": "IdentifierExpr",
+                      "layout": [
+                        {
+                          "id": 31,
+                          "tokenKind": {
+                            "kind": "identifier",
+                            "text": "xx"
+                          },
+                          "leadingTrivia": [],
+                          "trailingTrivia": [],
+                          "presence": "Present"
+                        },
+                        null
+                      ],
+                      "presence": "Present"
+                    }
+                  ],
+                  "presence": "Present"
+                }
+              ],
+              "presence": "Present"
+            },
+            null,
+            null
+          ],
+          "presence": "Present"
+        }
+      ],
+      "presence": "Present"
+    },
+    {
+      "id": 37,
+      "tokenKind": {
+        "kind": "eof",
+        "text": ""
+      },
+      "leadingTrivia": [],
+      "trailingTrivia": [],
+      "presence": "Present"
+    }
+  ],
+  "presence": "Present"
+}

test/incrParse/extend-identifier-at-eof.swift

@@ -0,0 +1,6 @@
+// RUN: %incr-transfer-tree --expected-incremental-syntax-tree %S/Outputs/extend-identifier-at-eof.json %s
+
+func foo() {}
+// ATTENTION: This file is testing the EOF token. 
+// DO NOT PUT ANYTHING AFTER THE CHANGE, NOT EVEN A NEWLINE
+_ = x<<<|||x>>>

test/incrParse/multi-edit-mapping.swift

@@ -0,0 +1,4 @@
+// RUN: %empty-directory(%t)
+// RUN: %validate-incrparse %s --test-case MULTI
+
+let one: Int;<reparse MULTI>let two: Int; let three: Int; <<MULTI<|||                      >>><<MULTI<|||   >>>let found: Int;</reparse MULTI>let five: Int;

test/incrParse/simple.swift

@@ -13,15 +13,16 @@
 // RUN: %validate-incrparse %s --test-case LAST_CHARACTER_OF_STRUCT
 // RUN: %validate-incrparse %s --test-case ADD_ARRAY_CLOSE_BRACKET
 // RUN: %validate-incrparse %s --test-case ADD_IF_OPEN_BRACE
+// RUN: %validate-incrparse %s --test-case EXTEND_IDENTIFIER
 
 func start() {}
 
 <reparse REPLACE>
 func foo() {
 }
 
-_ = <<REPLACE<6|||7>>></reparse REPLACE>
-_ = <<REPLACE_BY_LONGER<6|||"Hello World">>>
+_ = <<REPLACE<6|||7>>>
+_ = <<REPLACE_BY_LONGER<6|||"Hello World">>></reparse REPLACE>
 _ = <<REPLACE_BY_SHORTER<"Hello again"|||"a">>>
 <<INSERT<|||foo()>>>
 <<REMOVE<print("abc")|||>>>
@@ -52,3 +53,5 @@ var computedVar: [Int] {
 if true <<ADD_IF_OPEN_BRACE<|||{>>>
   _ = 5
 }
+
+let y<<EXTEND_IDENTIFIER<|||ou>>> = 42

tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

@@ -1996,7 +1996,7 @@ static unsigned resolveFromLineCol(unsigned Line, unsigned Col,
     exit(1);
   }
   Ptr = LineStart;
-  for (; Ptr < End; ++Ptr) {
+  for (; Ptr <= End; ++Ptr) {
     --Col;
     if (Col == 0)
       return Ptr - InputBuf->getBufferStart();

tools/swift-syntax-test/swift-syntax-test.cpp

@@ -246,11 +246,10 @@ struct ByteBasedSourceRange {
 
   bool empty() { return Start == End; }
 
-  SourceRange toSourceRange(SourceManager &SourceMgr, unsigned BufferID) {
+  CharSourceRange toCharSourceRange(SourceManager &SourceMgr, unsigned BufferID) {
     auto StartLoc = SourceMgr.getLocForOffset(BufferID, Start);
-    // SourceRange includes the last offset, we don't. So subtract 1
-    auto EndLoc = SourceMgr.getLocForOffset(BufferID, End - 1);
-    return SourceRange(StartLoc, EndLoc);
+    auto EndLoc = SourceMgr.getLocForOffset(BufferID, End);
+    return CharSourceRange(SourceMgr, StartLoc, EndLoc);
   }
 };
 
@@ -539,12 +538,11 @@ bool verifyReusedRegions(ByteBasedSourceRangeSet ExpectedReparseRegions,
   bool NoUnexpectedParse = true;
 
   for (auto ReparseRegion : UnexpectedReparseRegions.Ranges) {
-    auto ReparseRange = ReparseRegion.toSourceRange(SourceMgr, BufferID);
+    auto ReparseRange = ReparseRegion.toCharSourceRange(SourceMgr, BufferID);
 
     // To improve the ergonomics when writing tests we do not want to complain
     // about reparsed whitespaces.
-    auto RangeStr =
-        CharSourceRange(SourceMgr, ReparseRange.Start, ReparseRange.End).str();
+    auto RangeStr = ReparseRange.str();
     llvm::Regex WhitespaceOnlyRegex("^[ \t\r\n]*$");
     if (WhitespaceOnlyRegex.match(RangeStr)) {
       continue;

unittests/Parse/CMakeLists.txt

@@ -2,6 +2,7 @@ add_swift_unittest(SwiftParseTests
   BuildConfigTests.cpp
   LexerTests.cpp
   LexerTriviaTests.cpp
+  SyntaxParsingCacheTests.cpp
   TokenizerTests.cpp
 )