[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
Thu Apr 4 23:52:15 PDT 2024


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

>From eabedb36dd3d97edc0973321470ac9f69b64f572 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 end of CursorPlaceholder
 in formatting

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 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  |  7 ++++---
 clang/lib/Tooling/Core/Replacement.cpp        |  7 +++++--
 4 files changed, 29 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..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;
     }



More information about the cfe-commits mailing list