[clang][Rewriter] Adjust end offset before RewriteBuffer::getMappedOffset() call

[ArcsinX]

Without this patch, only cases when a token length increased were supported.
If a token lenght decreased, we returned a larger string than expected (e.g. in the added tests, "xretur " would be returned instead of "xretur")

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aleksandr Platonov (ArcsinX)

Changes

Without this patch, only cases when a token length increased were supported.
If a token lenght decreased, we returned a larger string than expected (e.g. in the added tests, "xretur " would be returned instead of "xretur")

---

Full diff: https://github.com/llvm/llvm-project/pull/187374.diff

2 Files Affected:

• (modified) clang/lib/Rewrite/Rewriter.cpp (+4-4)
• (modified) clang/unittests/Rewrite/RewriterTest.cpp (+8)

diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp
index a06fefaa5f188..6a29c8354da5b 100644
--- a/clang/lib/Rewrite/Rewriter.cpp
+++ b/clang/lib/Rewrite/Rewriter.cpp
@@ -112,15 +112,15 @@ std::string Rewriter::getRewrittenText(CharSourceRange Range) const {
     return std::string(Ptr, Ptr+EndOff-StartOff);
   }
 
-  const RewriteBuffer &RB = I->second;
-  EndOff = RB.getMappedOffset(EndOff, true);
-  StartOff = RB.getMappedOffset(StartOff);
-
   // Adjust the end offset to the end of the last token, instead of being the
   // start of the last token.
   if (Range.isTokenRange())
     EndOff += Lexer::MeasureTokenLength(Range.getEnd(), *SourceMgr, *LangOpts);
 
+  const RewriteBuffer &RB = I->second;
+  EndOff = RB.getMappedOffset(EndOff, true);
+  StartOff = RB.getMappedOffset(StartOff);
+
   // Advance the iterators to the right spot, yay for linear time algorithms.
   RewriteBuffer::iterator Start = RB.begin();
   std::advance(Start, StartOff);
diff --git a/clang/unittests/Rewrite/RewriterTest.cpp b/clang/unittests/Rewrite/RewriterTest.cpp
index ca72dde46fda7..74af36ea29532 100644
--- a/clang/unittests/Rewrite/RewriterTest.cpp
+++ b/clang/unittests/Rewrite/RewriterTest.cpp
@@ -48,6 +48,10 @@ TEST(Rewriter, GetRewrittenTextRangeTypes) {
   //              get char range ^~~    = "xret"
   //             get token range ^~~+++ = "xreturn"
   //            get source range ^~~+++ = "xreturn"
+  //                  remove "n"
+  //              get char range ^~~    = "xret"
+  //             get token range ^~~++  = "xretur"
+  //            get source range ^~~++  = "xretur"
   RangeTypeTest T(Code, 13, 16);
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.CRange), "ret");
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.TRange), "return");
@@ -56,6 +60,10 @@ TEST(Rewriter, GetRewrittenTextRangeTypes) {
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.CRange), "xret");
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.TRange), "xreturn");
   EXPECT_EQ(T.Rewrite.getRewrittenText(T.SRange), "xreturn");
+  T.Rewrite.RemoveText(T.makeLoc(18));
+  EXPECT_EQ(T.Rewrite.getRewrittenText(T.CRange), "xret");
+  EXPECT_EQ(T.Rewrite.getRewrittenText(T.TRange), "xretur");
+  EXPECT_EQ(T.Rewrite.getRewrittenText(T.SRange), "xretur");
 }
 
 TEST(Rewriter, ReplaceTextRangeTypes) {

[ArcsinX]

Friendly ping

[jansvoboda11]

Makes sense to me, but I'm not super familiar with this piece of code.