Add disabled unittest reproducing TextProto formatting issue.

[Icantjuddle]

Summary

Github Issue

Basically adding an empty TextProto section causes the formatting of TextProto raw strings to change and spaces to be added after the : .
This diff adds a test that reproduces this issus.
It is currently disabled as it is failing, here is the output:

❯ build/tools/clang/unittests/Format/FormatTests --gtest_filter='FormatTestRawStrings.DISABLED*' --gtest_also_run_disabled_tests
Note: Google Test filter = FormatTestRawStrings.DISABLED*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from FormatTestRawStrings
[ RUN      ] FormatTestRawStrings.DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection
/home/bjudd/code/llvm-project/clang/unittests/Format/FormatTestRawStrings.cpp:134: Failure
Expected equality of these values:
  Expected
    Which is: "\nt = R\"pb(item: 1)pb\";"
  Actual
    Which is: "\nt = R\"pb(item : 1)pb\";"
With diff:
@@ -1,2 +1,2 @@

-t = R\"pb(item: 1)pb\";
+t = R\"pb(item : 1)pb\";

[  FAILED  ] FormatTestRawStrings.DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection (7 ms)
[----------] 1 test from FormatTestRawStrings (7 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (7 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] FormatTestRawStrings.DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection

 1 FAILED TEST

[craig.topper]

I don't think I'm the right reviewer for this.

[Icantjuddle]

Sorry @craig.topper for some reason git blame locally attributed much of the edits to you, doesn't match what I see online at github.

[HazardyKnusperkeks]

As stated on Github I don't think we have a bug.

clang/unittests/Format/FormatTestRawStrings.cpp

79

Why such a raw string, instead of just "(...)"?

79

I think StringRef would be the correct type.

92–99

Couldn't you just set the style like you want, instead of parsing yaml?

177

EXPECT_EQ

[krasimir]

Thank you! I believe that's a bug in the raw string format manager.
This code effectively first looks for a matching top-level style, and if that's not found, then it tries to derive one via the RawFormat's BasedOnStyle:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Format/ContinuationIndenter.cpp#L191-L201

This doesn't match the documentation of the BasedOnStyle field, which indicates that, if specified, it should take precedence:
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Format/Format.h#L3219-L3222

@Icantjuddle would you like to update this patch to fix the issue and enable your newly added tests?

[MyDeveloperDay]

We don't normally land broken tests, even if they are disabled, its better for us if we get a fix at the same time ;-)

[Icantjuddle]

In D151145#4369406, @MyDeveloperDay wrote:

We don't normally land broken tests, even if they are disabled, its better for us if we get a fix at the same time ;-)

In D151145#4367495, @krasimir wrote:

...
@Icantjuddle would you like to update this patch to fix the issue and enable your newly added tests?

Sounds like we don't really want this without a real fix; I'll give it a shot, hopefully won't be too bad. Thanks for the code pointer.

In D151145#4365580, @HazardyKnusperkeks wrote:

As stated on Github I don't think we have a bug.

Are you sure, I'm pretty sure this is a bug? Can you elaborate on the discussion on github. I wouldn't want to fix something that isn't broken. If this is indeed expected maybe I can update the docs.

clang/unittests/Format/FormatTestRawStrings.cpp

92–99

Given that the problem seems to be with how the configs are parsed/processed, I wanted the repro to be most faithful to the user facing issue.

177

I used the function instead of the macro to conform to the convention of every other test in the file.

The function provides its own justification.

// Gcc 4.8 doesn't support raw string literals in macros, which breaks some
// build bots. We use this function instead.
void expect_eq(const std::string Expected, const std::string Actual) {

[HazardyKnusperkeks]

In D151145#4365580, @HazardyKnusperkeks wrote:

As stated on Github I don't think we have a bug.

Are you sure, I'm pretty sure this is a bug? Can you elaborate on the discussion on github. I wouldn't want to fix something that isn't broken. If this is indeed expected maybe I can update the docs.

Everything good, seems to be a bug.

clang/unittests/Format/FormatTestRawStrings.cpp

177

And every other clang-format test file (at least I worked on) uses the macro.

Also I don't think gcc 4.8 will be able to compile LLVM anymore, but I may be wrong on that.

clang/unittests/Format/FormatTestRawStrings.cpp

protected:
   FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
     return getStyleWithColumns(getLLVMStyle(), ColumnLimit);
   }
 
   int ReplacementCount;
 
   FormatStyle getRawStringPbStyleWithColumns(unsigned ColumnLimit) {
     FormatStyle Style = getLLVMStyle();
-    Style.ColumnLimit = ColumnLimit;
-    Style.RawStringFormats = {
-        {
-            /*Language=*/FormatStyle::LK_TextProto,
-            /*Delimiters=*/{"pb"},
-            /*EnclosingFunctions=*/{},
-            /*CanonicalDelimiter=*/"",
-            /*BasedOnStyle=*/"google",
-        },
-    };
+    Style.ColumnLimit = ColumnLimit;
+    Style.RawStringFormats = {
+        {
+            /*Language=*/FormatStyle::LK_TextProto,
+            /*Delimiters=*/{"pb"},
+            /*EnclosingFunctions=*/{},
+            /*CanonicalDelimiter=*/"",
+            /*BasedOnStyle=*/"google",
+        },
+    };
+    return Style;
+  }
+
+  FormatStyle getRawStringPbStyleSeparateSection() {
+    constexpr const char *FormatStr = R"format(---
+BasedOnStyle: LLVM
+ColumnLimit: 40
+RawStringFormats:
+  - Language: TextProto
+    Delimiters:
+      - 'pb'
+    BasedOnStyle: google
+---
+Language: TextProto
+# Don't set any options.
+...
+)format";
+    llvm::MemoryBufferRef ConfigBuffer(FormatStr, "ClangFormatConfig");
+    FormatStyle Style = {};
+    Style.Language = FormatStyle::LK_Cpp;
+    if (auto Err = parseConfiguration(ConfigBuffer, &Style)) {
+      llvm::dbgs() << Err.value();
+      llvm::dbgs() << Err.message();
+      assert(Err);
+    }
     return Style;
   }
 
   FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) {
     FormatStyle Style = getLLVMStyle();
     Style.RawStringFormats = {
         {
             /*Language=*/FormatStyle::LK_Cpp,
TEST_F(FormatTestRawStrings, UsesConfigurationOverBaseStyle) {
 
   // Uses the configured google style inside raw strings even if BasedOnStyle in
   // the raw string format is llvm.
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ(0, parseConfiguration("---\n"
                                   "Language: Cpp\n"
                                   "BasedOnStyle: Google",
                                   &Style)
-                   .value());
-  Style.RawStringFormats = {{
-      FormatStyle::LK_Cpp,
-      {"cpp"},
-      {},
-      /*CanonicalDelimiter=*/"",
-      /*BasedOnStyle=*/"llvm",
-  }};
-  expect_eq(R"test(int* i = R"cpp(int* j = 0;)cpp";)test",
-            format(R"test(int * i = R"cpp(int * j = 0;)cpp";)test", Style));
-}
-
-TEST_F(FormatTestRawStrings, MatchesDelimitersCaseSensitively) {
-  // Don't touch the 'PB' raw string, format the 'pb' raw string.
-  expect_eq(R"test(
-s = R"PB(item:1)PB";
-t = R"pb(item: 1)pb";)test",
-            format(R"test(
-s = R"PB(item:1)PB";
+                   .value());
+  Style.RawStringFormats = {{
+      FormatStyle::LK_Cpp,
+      {"cpp"},
+      {},
+      /*CanonicalDelimiter=*/"",
+      /*BasedOnStyle=*/"llvm",
+  }};
+  expect_eq(R"test(int* i = R"cpp(int* j = 0;)cpp";)test",
+            format(R"test(int * i = R"cpp(int * j = 0;)cpp";)test", Style));
+}
+
+TEST_F(FormatTestRawStrings, MatchesDelimitersCaseSensitively) {
+  // Don't touch the 'PB' raw string, format the 'pb' raw string.
+  expect_eq(R"test(
+s = R"PB(item:1)PB";
+t = R"pb(item: 1)pb";)test",
+            format(R"test(
+s = R"PB(item:1)PB";
+t = R"pb(item:1)pb";)test",
+                   getRawStringPbStyleWithColumns(40)));
+}
+
+// Currently a bug in how configs are processed:
+// See: https://github.com/llvm/llvm-project/issues/62871
+TEST_F(FormatTestRawStrings,
+       DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection) {
+  // TextProto formatting works when in a RawString.
+  expect_eq(R"test(
+t = R"pb(item: 1)pb";)test",
+            format(R"test(
 t = R"pb(item:1)pb";)test",
-                   getRawStringPbStyleWithColumns(40)));
+                   getRawStringPbStyleSeparateSection()));
 }
 
 TEST_F(FormatTestRawStrings, RespectsClangFormatOff) {
   expect_eq(R"test(
 // clang-format off
 s = R"pb(item:      1)pb";
 // clang-format on
 t = R"pb(item: 1)pb";)test",