Skip to content

Conversation

@owenca
Copy link
Contributor

@owenca owenca commented Oct 18, 2024

Fixes #108556.

@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #108556.


Full diff: https://github.com/llvm/llvm-project/pull/112839.diff

2 Files Affected:

  • (added) clang/test/Format/dry-run-warning.cpp (+12)
  • (modified) clang/tools/clang-format/ClangFormat.cpp (+7-6)
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(

@owenca owenca merged commit 85df281 into llvm:main Oct 19, 2024
6 of 8 checks passed
@owenca owenca deleted the 108556 branch October 19, 2024 04:10
@llvm llvm deleted a comment from llvm-ci Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clang-format -n xyz.json -> warning: code should be clang-formatted [-Wclang-format-violations]

3 participants