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

via cfe-commits cfe-commits at lists.llvm.org
Sat Apr 6 07:25:43 PDT 2024


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

>From b89d456b4ef5ae2311e448b939639c59b9c1aa23 Mon Sep 17 00:00:00 2001
From: roife <roifewu at gmail.com>
Date: Thu, 4 Apr 2024 14:54:08 +0800
Subject: [PATCH] [clangd] Fix: exclude insertions at the end of placeholder

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 be counted in. Otherwise, the insertion will not be counted in.

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

Fixes #71983
---
 clang-tools-extra/clangd/Format.cpp           |  2 +-
 .../clangd/unittests/FormatTests.cpp          | 19 +++++++++++++++++++
 .../include/clang/Tooling/Core/Replacement.h  |  8 +++++---
 clang/lib/Tooling/Core/Replacement.cpp        |  7 +++++--
 4 files changed, 30 insertions(+), 6 deletions(-)

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..72e662a07062d3 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..d2b79e4ab07ae7 100644
--- a/clang/include/clang/Tooling/Core/Replacement.h
+++ b/clang/include/clang/Tooling/Core/Replacement.h
@@ -268,9 +268,11 @@ 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..07d3e4e58718cb 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;
     }



More information about the cfe-commits mailing list