[incrParse] Fix SyntaxParsingCache::translateToPreEditPosition()

include/swift/Parse/SyntaxParsingCache.h

@@ -34,8 +34,11 @@ struct SourceEdit {
   /// The length of the string that replaced the range described above.
   size_t ReplacementLength;
 
+  SourceEdit(size_t Start, size_t End, size_t ReplacementLength)
+      : Start(Start), End(End), ReplacementLength(ReplacementLength){};
+
   /// The length of the range that has been replaced
-  size_t originalLength() { return End - Start; }
+  size_t originalLength() const { return End - Start; }
 
   /// Check if the characters replaced by this edit fall into the given range
   /// or are directly adjacent to it
@@ -65,12 +68,16 @@ class SyntaxParsingCache {
       : OldSyntaxTree(OldSyntaxTree) {}
 
   /// Add an edit that transformed the source file which created this cache into
-  /// the source file that is now being parsed incrementally. The order in which
-  /// the edits are added using this method needs to be the same order in which
-  /// the edits were applied to the source file.
-  void addEdit(size_t Start, size_t End, size_t ReplacementLength) {
-    Edits.push_back({Start, End, ReplacementLength});
-  }
+  /// the source file that is now being parsed incrementally. \c Start must be a
+  /// position from the *original* source file, and it must not overlap any
+  /// other edits previously added. For instance, given:
+  ///   (aaa, bbb)
+  ///   0123456789
+  /// When you want to turn this into:
+  ///   (c, dddd)
+  ///   0123456789
+  /// edits should be: { 1, 4, 1 } and { 6, 9, 4 }.
+  void addEdit(size_t Start, size_t End, size_t ReplacementLength);
 
   /// Check if a syntax node of the given kind at the given position can be
   /// reused for a new syntax tree.
@@ -86,11 +93,13 @@ class SyntaxParsingCache {
   getReusedRegions(const SourceFileSyntax &SyntaxTree) const;
 
   /// Translates a post-edit position to a pre-edit position by undoing the
-  /// specified edits.
+  /// specified edits. Returns \c None if no pre-edit position exists because
+  /// the post-edit position has been inserted by an edit.
+  /// 
   /// Should not be invoked externally. Only public for testing purposes.
-  static size_t
+  static Optional<size_t>
   translateToPreEditPosition(size_t PostEditPosition,
-                             llvm::SmallVector<SourceEdit, 4> Edits);
+                             ArrayRef<SourceEdit> Edits);
 
 private:
   llvm::Optional<Syntax> lookUpFrom(const Syntax &Node, size_t NodeStart,

lib/Parse/SyntaxParsingCache.cpp

@@ -16,6 +16,13 @@
 using namespace swift;
 using namespace swift::syntax;
 
+void SyntaxParsingCache::addEdit(size_t Start, size_t End,
+                                 size_t ReplacementLength) {
+  assert((Edits.empty() || Edits.back().End <= Start) &&
+         "'Start' must be greater than or equal to 'End' of the previous edit");
+  Edits.emplace_back(Start, End, ReplacementLength);
+}
+
 bool SyntaxParsingCache::nodeCanBeReused(const Syntax &Node, size_t NodeStart,
                                          size_t Position,
                                          SyntaxKind Kind) const {
@@ -80,23 +87,31 @@ llvm::Optional<Syntax> SyntaxParsingCache::lookUpFrom(const Syntax &Node,
   return llvm::None;
 }
 
-size_t SyntaxParsingCache::translateToPreEditPosition(
-    size_t PostEditPosition, llvm::SmallVector<SourceEdit, 4> Edits) {
+Optional<size_t>
+SyntaxParsingCache::translateToPreEditPosition(size_t PostEditPosition,
+                                               ArrayRef<SourceEdit> Edits) {
   size_t Position = PostEditPosition;
-  for (auto I = Edits.begin(), E = Edits.end(); I != E; ++I) {
-    auto Edit = *I;
-    if (Edit.End + Edit.ReplacementLength - Edit.originalLength() <= Position) {
-      Position = Position - Edit.ReplacementLength + Edit.originalLength();
-    }
+  for (auto &Edit : Edits) {
+    if (Edit.Start > Position)
+      // Remaining edits doesn't affect the position. (Edits are sorted)
+      break;
+    if (Edit.Start + Edit.ReplacementLength > Position)
+      // This is a position inserted by the edit, and thus doesn't exist in the
+      // pre-edit version of the file.
+      return None;
+
+    Position = Position - Edit.ReplacementLength + Edit.originalLength();
   }
   return Position;
 }
 
 llvm::Optional<Syntax> SyntaxParsingCache::lookUp(size_t NewPosition,
                                                   SyntaxKind Kind) {
-  size_t OldPosition = translateToPreEditPosition(NewPosition, Edits);
+  Optional<size_t> OldPosition = translateToPreEditPosition(NewPosition, Edits);
+  if (!OldPosition.hasValue())
+    return None;
 
-  auto Node = lookUpFrom(OldSyntaxTree, /*NodeStart=*/0, OldPosition, Kind);
+  auto Node = lookUpFrom(OldSyntaxTree, /*NodeStart=*/0, *OldPosition, Kind);
   if (Node.hasValue()) {
     ReusedNodeIds.insert(Node->getId());
   }

test/incrParse/inserted_text_starts_identifier.swift

@@ -0,0 +1,6 @@
+// RUN: %empty-directory(%t)
+// RUN: %validate-incrparse %s --test-case STRING
+
+// SR-8995 rdar://problem/45259469
+
+self = <<STRING<|||_                            _>>>foo(1)[object1, object2] + o bar(1)

unittests/Parse/SyntaxParsingCacheTests.cpp

@@ -1,119 +1,105 @@
 #include "swift/Parse/SyntaxParsingCache.h"
 #include "gtest/gtest.h"
 
+#include <iostream>
+
 using namespace swift;
 using namespace llvm;
 
+
+namespace llvm {
+template <typename T>
+void PrintTo(const Optional<T> &optVal, ::std::ostream *os) {
+  if (optVal.hasValue())
+    *os << *optVal;
+  else
+    *os << "None";
+}
+} // namespace llvm
+
+void check(ArrayRef<SourceEdit> Edits, ArrayRef<Optional<size_t>> expected) {
+  for (size_t Pos = 0; Pos != expected.size(); ++Pos) {
+    Optional<size_t> PrePos =
+        SyntaxParsingCache::translateToPreEditPosition(Pos, Edits);
+    EXPECT_EQ(PrePos, expected[Pos]) << "At post-edit position " << Pos;
+  }
+}
+
 class TranslateToPreEditPositionTest : public ::testing::Test {};
 
-TEST_F(TranslateToPreEditPositionTest, SingleEditBefore) {
+TEST_F(TranslateToPreEditPositionTest, SingleEdit1) {
   // Old: ab_xy
-  // New: a1b_xy
-  //
-  // Edits:
-  // (1) 1-2: a -> a1
-  //
-  // Lookup for _ at new position 4
+  // New: c_xy
 
   llvm::SmallVector<SourceEdit, 4> Edits = {
-      {1, 2, 2}
+      {0, 2, 1} // ab -> c
   };
 
-  size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(4, Edits);
-  EXPECT_EQ(PreEditPos, 3u);
+  //            c     _  x  y
+  check(Edits, {None, 2, 3, 4});
 }
 
-TEST_F(TranslateToPreEditPositionTest, SingleEditDirectlyBefore) {
+TEST_F(TranslateToPreEditPositionTest, SingleEdit) {
   // Old: ab_xy
   // New: ablah_xy
-  //
-  // Edits:
-  // (1) 2-3: b -> blah
-  //
-  // Lookup for _ at new position 6
 
   llvm::SmallVector<SourceEdit, 4> Edits = {
-      {2, 3, 4}
+      {1, 2, 4} // b -> blah
   };
 
-  size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(6, Edits);
-  EXPECT_EQ(PreEditPos, 3u);
+  //            a  b     l     a     h     _  x  y
+  check(Edits, {0, None, None, None, None, 2, 3, 4});
 }
 
-TEST_F(TranslateToPreEditPositionTest, SingleMultiCharacterEdit) {
+TEST_F(TranslateToPreEditPositionTest, SingleInsert) {
   // Old: ab_xy
-  // New: abcdef_xy
-  //
-  // Edits:
-  // (1) 1-3: ab -> abcdef
-  //
-  // Lookup for _ at new position 7
+  // New: 0123ab_xy
 
   llvm::SmallVector<SourceEdit, 4> Edits = {
-      {1, 3, 6}
+      {0, 0, 4} // '' -> 0123
   };
 
-  size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(7, Edits);
-  EXPECT_EQ(PreEditPos, 3u);
+  //             0     1     2     3     a  b  _  x  y
+  check(Edits, { None, None, None, None, 0, 1, 2, 3, 4});
 }
 
-TEST_F(TranslateToPreEditPositionTest, EditAfterLookup) {
-  // Old: ab_xy
-  // New: ab_xyz
-  //
-  // Edits:
-  // (1) 4-6: xy -> xyz
-  //
-  // Lookup for _ at new position 3
+TEST_F(TranslateToPreEditPositionTest, SingleDelete) {
+  // Old: ab_xyz
+  // New: ab_z
 
-  llvm::SmallVector<SourceEdit, 4> Edits = {{4, 6, 4}};
+  llvm::SmallVector<SourceEdit, 4> Edits = {
+      {3, 5, 0} // xy -> ''
+  };
 
-  size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(3, Edits);
-  EXPECT_EQ(PreEditPos, 3u);
+  //             a  b  _  z
+  check(Edits, { 0, 1, 2, 5 });
 }
 
 TEST_F(TranslateToPreEditPositionTest, SimpleMultiEdit) {
-  // Old: ab_xy
-  // New: a1b2_x3y4
-  //
-  // Edits:
-  // (1) 1-2: a -> a1
-  // (2) 2-3: b -> b2
-  // (3) 4-5: x -> x3
-  // (4) 5-6: y -> y4
-  //
-  // Lookup for _ at new position 5
+  // Old: _ab_xy
+  // New: _a1b2_x3y4
 
   llvm::SmallVector<SourceEdit, 4> Edits = {
-      {1, 2, 2},
-      {2, 3, 2},
-      {4, 5, 2},
-      {5, 6, 2},
+      {1, 2, 2}, // a -> a1
+      {2, 3, 2}, // b -> b2
+      {4, 5, 2}, // x -> x3
+      {5, 6, 2}, // y -> y4
   };
 
-  size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(5, Edits);
-  EXPECT_EQ(PreEditPos, 3u);
+  //            _  a     1     b     1     _  x     3     y     4
+  check(Edits, {0, None, None, None, None, 3, None, None, None, None});
 }
 
-TEST_F(TranslateToPreEditPositionTest, LongMultiEdit) {
-  // Old: ab_xy
-  // New: a11111b2_x3y4
-  //
-  // Edits:
-  // (1) 1-2: a -> a11111
-  // (2) 2-3: b -> b2
-  // (3) 4-5: x -> x3
-  // (4) 5-6: y -> y4
-  //
-  // Lookup for _ at new position
+TEST_F(TranslateToPreEditPositionTest, ComplexMultiEdit) {
+  // Old: foo_bar_baz
+  // New: xx_edits_baz
 
   llvm::SmallVector<SourceEdit, 4> Edits = {
-      {1, 2, 6},
-      {2, 3, 2},
-      {4, 5, 2},
-      {5, 6, 2},
+      {0, 3, 2}, // foo -> xx
+      {4, 7, 0}, // bar -> ''
+      {7, 7, 5}, // '' -> edits
   };
 
-  size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(9, Edits);
-  EXPECT_EQ(PreEditPos, 3u);
-}
+  //            x     x     _  e     d     i     t     s     _  b  a  z
+  check(Edits, {None, None, 3, None, None, None, None, None, 7, 8, 9, 10});
+}