[clangd] Handle tabs in getIncrementalChangesAfterNewline()

Tabs are not handled by columnWidthUTF8() (they are considered
non-printable) so require additional logic to handle.

Fixes https://github.com/clangd/clangd/issues/1074

Differential Revision: https://reviews.llvm.org/D122554

diff --git a/clang-tools-extra/clangd/Format.cpp b/clang-tools-extra/clangd/Format.cpp
index bb492db..c3e9263 100644 (file)
--- a/clang-tools-extra/clangd/Format.cpp
+++ b/clang-tools-extra/clangd/Format.cpp
@@ -116,12 +116,41 @@ struct IncrementalChanges {
   std::string CursorPlaceholder;
 };
 
+// The two functions below, columnWidth() and columnWidthWithTabs(), were
+// adapted from similar functions in clang/lib/Format/Encoding.h.
+// FIXME: Move those functions to clang/include/clang/Format.h and reuse them?
+
+// Helper function for columnWidthWithTabs().
+inline unsigned columnWidth(StringRef Text) {
+  int ContentWidth = llvm::sys::unicode::columnWidthUTF8(Text);
+  if (ContentWidth < 0)
+    return Text.size(); // fallback for unprintable characters
+  return ContentWidth;
+}
+
+// Returns the number of columns required to display the \p Text on a terminal
+// with the \p TabWidth.
+inline unsigned columnWidthWithTabs(StringRef Text, unsigned TabWidth) {
+  unsigned TotalWidth = 0;
+  StringRef Tail = Text;
+  for (;;) {
+    StringRef::size_type TabPos = Tail.find('\t');
+    if (TabPos == StringRef::npos)
+      return TotalWidth + columnWidth(Tail);
+    TotalWidth += columnWidth(Tail.substr(0, TabPos));
+    if (TabWidth)
+      TotalWidth += TabWidth - TotalWidth % TabWidth;
+    Tail = Tail.substr(TabPos + 1);
+  }
+}
+
 // After a newline:
 //  - we continue any line-comment that was split
 //  - we format the old line in addition to the cursor
 //  - we represent the cursor with a line comment to preserve the newline
 IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
-                                                     unsigned Cursor) {
+                                                     unsigned Cursor,
+                                                     unsigned TabWidth) {
   IncrementalChanges Result;
   // Before newline, code looked like:
   //    leading^trailing
@@ -152,12 +181,12 @@ IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
   if (!CommentMarker.empty() &&
       (NewLineIsComment || !commentMarker(NextLine).empty() ||
        (!TrailingTrim.empty() && !TrailingTrim.startswith("//")))) {
-    using llvm::sys::unicode::columnWidthUTF8;
     // We indent the new comment to match the previous one.
     StringRef PreComment =
         Leading.take_front(CommentMarker.data() - Leading.data());
     std::string IndentAndComment =
-        (std::string(columnWidthUTF8(PreComment), ' ') + CommentMarker + " ")
+        (std::string(columnWidthWithTabs(PreComment, TabWidth), ' ') +
+         CommentMarker + " ")
             .str();
     cantFail(
         Result.Changes.add(replacement(Code, Indentation, IndentAndComment)));
@@ -191,10 +220,11 @@ IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
 }
 
 IncrementalChanges getIncrementalChanges(llvm::StringRef Code, unsigned Cursor,
-                                         llvm::StringRef InsertedText) {
+                                         llvm::StringRef InsertedText,
+                                         unsigned TabWidth) {
   IncrementalChanges Result;
   if (InsertedText == "\n")
-    return getIncrementalChangesAfterNewline(Code, Cursor);
+    return getIncrementalChangesAfterNewline(Code, Cursor, TabWidth);
 
   Result.CursorPlaceholder = " /**/";
   return Result;
@@ -246,8 +276,8 @@ split(const tooling::Replacements &Replacements, unsigned OldCursor,
 std::vector<tooling::Replacement>
 formatIncremental(llvm::StringRef OriginalCode, unsigned OriginalCursor,
                   llvm::StringRef InsertedText, format::FormatStyle Style) {
-  IncrementalChanges Incremental =
-      getIncrementalChanges(OriginalCode, OriginalCursor, InsertedText);
+  IncrementalChanges Incremental = getIncrementalChanges(
+      OriginalCode, OriginalCursor, InsertedText, Style.TabWidth);
   // Never *remove* lines in response to pressing enter! This annoys users.
   if (InsertedText == "\n") {
     Style.MaxEmptyLinesToKeep = 1000;

diff --git a/clang-tools-extra/clangd/unittests/FormatTests.cpp b/clang-tools-extra/clangd/unittests/FormatTests.cpp
index 0eb217e..f7384a1 100644 (file)
--- a/clang-tools-extra/clangd/unittests/FormatTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FormatTests.cpp
@@ -19,13 +19,11 @@ namespace clang {
 namespace clangd {
 namespace {
 
-std::string afterTyped(llvm::StringRef CodeWithCursor,
-                           llvm::StringRef Typed) {
+std::string afterTyped(llvm::StringRef CodeWithCursor, llvm::StringRef Typed,
+                       clang::format::FormatStyle Style) {
   Annotations Code(CodeWithCursor);
   unsigned Cursor = llvm::cantFail(positionToOffset(Code.code(), Code.point()));
-  auto Changes =
-      formatIncremental(Code.code(), Cursor, Typed,
-                        format::getGoogleStyle(format::FormatStyle::LK_Cpp));
+  auto Changes = formatIncremental(Code.code(), Cursor, Typed, Style);
   tooling::Replacements Merged;
   for (const auto& R : Changes)
     if (llvm::Error E = Merged.add(R))
@@ -38,11 +36,15 @@ std::string afterTyped(llvm::StringRef CodeWithCursor,
 }
 
 // We can't pass raw strings directly to EXPECT_EQ because of gcc bugs.
-void expectAfterNewline(const char *Before, const char *After) {
-  EXPECT_EQ(After, afterTyped(Before, "\n")) << Before;
+void expectAfterNewline(const char *Before, const char *After,
+                        format::FormatStyle Style = format::getGoogleStyle(
+                            format::FormatStyle::LK_Cpp)) {
+  EXPECT_EQ(After, afterTyped(Before, "\n", Style)) << Before;
 }
-void expectAfter(const char *Typed, const char *Before, const char *After) {
-  EXPECT_EQ(After, afterTyped(Before, Typed)) << Before;
+void expectAfter(const char *Typed, const char *Before, const char *After,
+                 format::FormatStyle Style =
+                     format::getGoogleStyle(format::FormatStyle::LK_Cpp)) {
+  EXPECT_EQ(After, afterTyped(Before, Typed, Style)) << Before;
 }
 
 TEST(FormatIncremental, SplitComment) {
@@ -131,7 +133,7 @@ void foo() {
             ^
 }
 )cpp",
-   R"cpp(
+                     R"cpp(
 void foo() {
   if (x)
     return;  // All spelled tokens are accounted for.
@@ -139,6 +141,17 @@ void foo() {
   ^
 }
 )cpp");
+
+  // Handle tab character in leading indentation
+  format::FormatStyle TabStyle =
+      format::getGoogleStyle(format::FormatStyle::LK_Cpp);
+  TabStyle.UseTab = format::FormatStyle::UT_Always;
+  TabStyle.TabWidth = 4;
+  TabStyle.IndentWidth = 4;
+  // Do not use raw strings, otherwise '\t' will be interpreted literally.
+  expectAfterNewline("void foo() {\n\t// this comment was\n^split\n}\n",
+                     "void foo() {\n\t// this comment was\n\t// ^split\n}\n",
+                     TabStyle);
 }
 
 TEST(FormatIncremental, Indentation) {