[clang] [clang-format] Fix a bug that always returns error for JSON (PR #112839)

Owen Pan via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 17 22:06:36 PDT 2024


https://github.com/owenca created https://github.com/llvm/llvm-project/pull/112839

Fixes #108556.

>From 9eb81c845aa102e28c87eeefe82fac3f029ae29e Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Thu, 17 Oct 2024 21:52:16 -0700
Subject: [PATCH] [clang-format] Fix a bug that always returns error for JSON

Fixes #108556.
---
 clang/test/Format/dry-run-warning.cpp    | 12 ++++++++++++
 clang/tools/clang-format/ClangFormat.cpp | 13 +++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)
 create mode 100644 clang/test/Format/dry-run-warning.cpp

diff --git a/clang/test/Format/dry-run-warning.cpp b/clang/test/Format/dry-run-warning.cpp
new file mode 100644
index 00000000000000..d71ac9a328f4ee
--- /dev/null
+++ b/clang/test/Format/dry-run-warning.cpp
@@ -0,0 +1,12 @@
+// RUN: echo '{' > %t.json
+// RUN: echo '  "married": true' >> %t.json
+// RUN: echo '}' >> %t.json
+
+// RUN: clang-format -n -style=LLVM %t.json 2>&1 | FileCheck %s -allow-empty
+// CHECK-NOT: warning
+
+// RUN: clang-format -n -style=LLVM < %t.json 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=CHECK2 -strict-whitespace
+// CHECK2: warning: code should be clang-formatted
+
+// RUN: rm %t.json
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index 6aed46328f3469..a1a4d73dfe9eb5 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -351,9 +351,6 @@ static void outputReplacementsXML(const Replacements &Replaces) {
 static bool
 emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
                         const std::unique_ptr<llvm::MemoryBuffer> &Code) {
-  if (Replaces.empty())
-    return false;
-
   unsigned Errors = 0;
   if (WarnFormat && !NoWarnFormat) {
     SourceMgr Mgr;
@@ -490,9 +487,11 @@ static bool format(StringRef FileName, bool ErrorOnIncompleteFormat = false) {
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
                                        AssumedFileName, &CursorPosition);
 
+  const bool IsJson = FormatStyle->isJson();
+
   // To format JSON insert a variable to trick the code into thinking its
   // JavaScript.
-  if (FormatStyle->isJson() && !FormatStyle->DisableFormat) {
+  if (IsJson && !FormatStyle->DisableFormat) {
     auto Err = Replaces.add(tooling::Replacement(
         tooling::Replacement(AssumedFileName, 0, 0, "x = ")));
     if (Err)
@@ -511,8 +510,10 @@ static bool format(StringRef FileName, bool ErrorOnIncompleteFormat = false) {
       reformat(*FormatStyle, *ChangedCode, Ranges, AssumedFileName, &Status);
   Replaces = Replaces.merge(FormatChanges);
   if (OutputXML || DryRun) {
-    if (DryRun)
-      return emitReplacementWarnings(Replaces, AssumedFileName, Code);
+    if (DryRun) {
+      return (Replaces.size() > IsJson ? 1 : 0) &&
+             emitReplacementWarnings(Replaces, AssumedFileName, Code);
+    }
     outputXML(Replaces, FormatChanges, Status, Cursor, CursorPosition);
   } else {
     IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(



More information about the cfe-commits mailing list