[clangd] Handle tabs in getIncrementalChangesAfterNewline()

[nridge]

Summary

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

[sammccall]

Thanks!

Going to add tabs to case-sensitivity and symlinks to my list of things that will haunt me forever...

clang-tools-extra/clangd/Format.cpp

126

Comment "unprintable characters" or so?
(Maybe invert the condition for this purpose)

[nridge]

address review comment

clang-tools-extra/clangd/Format.cpp

struct IncrementalChanges {
   // The CursorProxyText will also be formatted.
   std::vector<tooling::Range> FormatRanges;
   // The source code that should stand in for the cursor when clang-formatting.
   // e.g. after inserting a newline, a line-comment at the cursor is used to
   // ensure that the newline is preserved.
   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
   // After newline, code looks like:
   //    leading
   //    indentation^trailing
   // Where indentation was added by the editor.
   StringRef Trailing = firstLine(Code.substr(Cursor));
IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
 
   // If we split a comment, replace indentation with a comment marker.
   // If the editor made the new line a comment, also respect that.
   StringRef CommentMarker = commentMarker(Leading);
   bool NewLineIsComment = !commentMarker(Indentation).empty();
   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)));
   } else {
     // Remove any indentation and let clang-format re-add it.
     // This prevents the cursor marker dragging e.g. an aligned comment with it.
     cantFail(Result.Changes.add(replacement(Code, Indentation, "")));
   }
IncrementalChanges getIncrementalChangesAfterNewline(llvm::StringRef Code,
   // Exception: if the previous line has a trailing comment, we can't use one
   // as the cursor (they will be aligned). But in this case we don't need to.
   Result.CursorPlaceholder = !CommentMarker.empty() ? "ident" : "//==\nident";
 
   return Result;
 }
 
 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;
 }
 
 // Returns equivalent replacements that preserve the correspondence between
 // OldCursor and NewCursor. If OldCursor lies in a replaced region, that
 // replacement will be split.

 // The replacements we return are produced by composing these.
 //
 // The text we actually pass to clang-format is slightly different from this,
 // e.g. we have to close brackets. We ensure these differences are *after*
 // all the regions we want to format, and discard changes in them.
 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;
     Style.KeepEmptyLinesAtTheStartOfBlocks = true;
   }
 
   // Compute the code we want to format:
   // 1) Start with code after the pre-formatting edits.

clang-tools-extra/clangd/unittests/FormatTests.cpp

 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 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))
       ADD_FAILURE() << llvm::toString(std::move(E));
   auto NewCode = tooling::applyAllReplacements(Code.code(), Merged);
   EXPECT_TRUE(bool(NewCode))
       << "Bad replacements: " << llvm::toString(NewCode.takeError());
   NewCode->insert(transformCursorPosition(Cursor, Changes), "^");
   return *NewCode;
 }
 
 // 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 expectAfter(const char *Typed, const char *Before, const char *After) {
-  EXPECT_EQ(After, afterTyped(Before, Typed)) << 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,
+                 format::FormatStyle Style =
+                     format::getGoogleStyle(format::FormatStyle::LK_Cpp)) {
+  EXPECT_EQ(After, afterTyped(Before, Typed, Style)) << Before;
 }
 
 TEST(FormatIncremental, SplitComment) {
   expectAfterNewline(R"cpp(
 // this comment was
 ^split
 )cpp",
    R"cpp(
)cpp");
   expectAfterNewline(R"cpp(
 void foo() {
   if (x)
     return; // All spelled tokens are accounted for.
             // that takes two lines
             ^
 }
 )cpp",
                      R"cpp(
 void foo() {
   if (x)
     return;  // All spelled tokens are accounted for.
              // that takes two lines
   ^
 }
 )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) {
   expectAfterNewline(R"cpp(
 void foo() {
   if (bar)
 ^
 )cpp",