[PATCH] D115769: [clang-format] Remove spurious JSON binding when DisableFormat = true

[Andrew Smith via Phabricator via cfe-commits]

Andrew-William-Smith updated this revision to Diff 394582.
Andrew-William-Smith added a comment.

Added a test with formatting disabled.  I had to port the updated logic from `ClangFormat.cpp` into `FormatTestJson::format` to show the desired effect; I also couldn't call `verifyFormat` as-is with formatting disabled since the second assertion involving `test::messUp` would modify the formatting of the input JSON with no way to format it back.  I've added a new function `FormatTestJson::verifyFormatStable` that only checks that formatting is stable for this scenario.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115769/new/

https://reviews.llvm.org/D115769

Files:
  clang/unittests/Format/FormatTestJson.cpp


Index: clang/unittests/Format/FormatTestJson.cpp
===================================================================
--- clang/unittests/Format/FormatTestJson.cpp
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -27,7 +27,7 @@
 
     // Mock up what ClangFormat.cpp will do for JSON by adding a variable
     // to trick JSON into being JavaScript
-    if (Style.isJson()) {
+    if (Style.isJson() && !Style.DisableFormat) {
       auto Err = Replaces.add(
           tooling::Replacement(tooling::Replacement("", 0, 0, "x = ")));
       if (Err) {
@@ -60,10 +60,15 @@
     return Style;
   }
 
+  static void verifyFormatStable(llvm::StringRef Code,
+                                 const FormatStyle &Style) {
+    EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+  }
+
   static void
   verifyFormat(llvm::StringRef Code,
                const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Json)) {
-    EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
+    verifyFormatStable(Code, Style);
     EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 };
@@ -193,5 +198,24 @@
                Style);
 }
 
+TEST_F(FormatTestJson, DisableJsonFormat) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  verifyFormatStable("{}", Style);
+  verifyFormatStable("{\n"
+                     "  \"name\": 1\n"
+                     "}",
+                     Style);
+
+  // Since we have to disable formatting to run this test, we shall refrain from
+  // calling test::messUp lest we change the unformatted code and cannot format
+  // it back to how it started.
+  Style.DisableFormat = true;
+  verifyFormatStable("{}", Style);
+  verifyFormatStable("{\n"
+                     "  \"name\": 1\n"
+                     "}",
+                     Style);
+}
+
 } // namespace format
 } // end namespace clang


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115769.394582.patch
Type: text/x-patch
Size: 1888 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211215/0bd78017/attachment.bin>