[clang] [clang-tools-extra] [clangd] Fix: exclude insertions at end of CursorPlaceholder in formatting (PR #87746)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: roife (roife)

<details>
<summary>Changes</summary>

Fixes #<!-- -->71983

This commit introduces a new param 'includeInsAtPos' for 'getShiftedCodePosition', which is defaulted to be true. If 'includeInsAtPos' is true, the insertion at the 'Position' will not be counted in.

Changes made:
- Added new param 'includeInsAtPos' to 'getShiftedCodePosition'.
- Set 'includeInsAtPos' to be true when calculate the end position of 'CursorPlaceholder' in 'formatIncremental' of clangd.

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


4 Files Affected:

- (modified) clang-tools-extra/clangd/Format.cpp (+1-1) 
- (modified) clang-tools-extra/clangd/unittests/FormatTests.cpp (+19) 
- (modified) clang/include/clang/Tooling/Core/Replacement.h (+4-3) 
- (modified) clang/lib/Tooling/Core/Replacement.cpp (+5-2) 


``````````diff
diff --git a/clang-tools-extra/clangd/Format.cpp b/clang-tools-extra/clangd/Format.cpp
index 272a34d4ed7972..901a709e2e3934 100644
--- a/clang-tools-extra/clangd/Format.cpp
+++ b/clang-tools-extra/clangd/Format.cpp
@@ -341,7 +341,7 @@ formatIncremental(llvm::StringRef OriginalCode, unsigned OriginalCursor,
   unsigned FormattedCursorStart =
                FormattingChanges.getShiftedCodePosition(Cursor),
            FormattedCursorEnd = FormattingChanges.getShiftedCodePosition(
-               Cursor + Incremental.CursorPlaceholder.size());
+               Cursor + Incremental.CursorPlaceholder.size(), false);
   tooling::Replacements RemoveCursorPlaceholder(
       tooling::Replacement(Filename, FormattedCursorStart,
                            FormattedCursorEnd - FormattedCursorStart, ""));
diff --git a/clang-tools-extra/clangd/unittests/FormatTests.cpp b/clang-tools-extra/clangd/unittests/FormatTests.cpp
index f7384a1bc63c99..22c19fefc61aad 100644
--- a/clang-tools-extra/clangd/unittests/FormatTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FormatTests.cpp
@@ -315,6 +315,25 @@ vector<int> x = {1, 2, 3}^
 )cpp");
 }
 
+TEST(FormatIncremental, InsertBraces) {
+  format::FormatStyle Style =
+    format::getGoogleStyle(format::FormatStyle::LK_Cpp);
+  Style.InsertBraces = true;
+  expectAfterNewline(R"cpp(
+int main() {
+  while (true)
+^
+}
+)cpp",
+                     R"cpp(
+int main() {
+  while (true) {
+    
+  }^
+}
+)cpp",
+                     Style);
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang/include/clang/Tooling/Core/Replacement.h b/clang/include/clang/Tooling/Core/Replacement.h
index f9452111e147f1..69fe5186dfb0a5 100644
--- a/clang/include/clang/Tooling/Core/Replacement.h
+++ b/clang/include/clang/Tooling/Core/Replacement.h
@@ -268,9 +268,10 @@ class Replacements {
   std::vector<Range> getAffectedRanges() const;
 
   // Returns the new offset in the code after replacements being applied.
-  // Note that if there is an insertion at Offset in the current replacements,
-  // \p Offset will be shifted to Offset + Length in inserted text.
-  unsigned getShiftedCodePosition(unsigned Position) const;
+  // If \p includeInsAtPos is true and there is an insertion at Offset in the
+  // current replacements, \p Offset will be shifted to Offset + Length in
+  // inserted text. Otherwise, the insertion at Offset will not be counted in.
+  unsigned getShiftedCodePosition(unsigned Position, bool includeInsAtPos = true) const;
 
   unsigned size() const { return Replaces.size(); }
 
diff --git a/clang/lib/Tooling/Core/Replacement.cpp b/clang/lib/Tooling/Core/Replacement.cpp
index 269f17a6db4cfc..95a3b62f47c708 100644
--- a/clang/lib/Tooling/Core/Replacement.cpp
+++ b/clang/lib/Tooling/Core/Replacement.cpp
@@ -544,10 +544,13 @@ std::vector<Range> Replacements::getAffectedRanges() const {
   return combineAndSortRanges(ChangedRanges);
 }
 
-unsigned Replacements::getShiftedCodePosition(unsigned Position) const {
+unsigned Replacements::getShiftedCodePosition(unsigned Position,
+                                              bool includeInsAtPos) const {
   unsigned Offset = 0;
   for (const auto &R : Replaces) {
-    if (R.getOffset() + R.getLength() <= Position) {
+    unsigned End = R.getOffset() + R.getLength();
+    if (End <= Position
+        && (includeInsAtPos || (End < Position || R.getLength() > 0))) {
       Offset += R.getReplacementText().size() - R.getLength();
       continue;
     }

``````````

</details>


https://github.com/llvm/llvm-project/pull/87746