[clang-tools-extra] Apply format only if --format is specified (PR #79466)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Dmitry Polukhin (dmpolukhin)

<details>
<summary>Changes</summary>

clang-apply-replacements used to apply format even without --format is specified. This because, methods like createReplacementsForHeaders only takes the Spec.Style and would re-order the headers even when it was not requested. The fix is to set up Spec.Style only if --format is provided.

Also added note to ReleaseNotes.rst

Based on https://github.com/llvm/llvm-project/pull/70801

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


7 Files Affected:

- (modified) clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp (+1-1) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+7) 
- (added) clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.cpp (+10) 
- (added) clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.yaml (+14) 
- (added) clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.cpp (+10) 
- (added) clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.yaml (+14) 
- (added) clang-tools-extra/test/clang-apply-replacements/format-header.cpp (+13) 


``````````diff
diff --git a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
index 9331898bf2570e6..68b5743c6540f8a 100644
--- a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
+++ b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
@@ -141,9 +141,9 @@ int main(int argc, char **argv) {
 
   tooling::ApplyChangesSpec Spec;
   Spec.Cleanup = true;
-  Spec.Style = FormatStyle;
   Spec.Format = DoFormat ? tooling::ApplyChangesSpec::kAll
                          : tooling::ApplyChangesSpec::kNone;
+  Spec.Style = DoFormat ? FormatStyle : format::getNoStyle();
 
   for (const auto &FileChange : Changes) {
     FileEntryRef Entry = FileChange.first;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fd2cba4e4f463be..228c9c176232161 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -109,6 +109,13 @@ Changes in existing checks
 Removed checks
 ^^^^^^^^^^^^^^
 
+Miscellaneous
+^^^^^^^^^^^^^
+
+- Fixed incorrect apply format in clang-apply-repalcements when no `--format`
+  option is specified. Now clang-apply-repalcements applies format only with
+  the option.
+
 Improvements to include-fixer
 -----------------------------
 
diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.cpp b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.cpp
new file mode 100644
index 000000000000000..140e266200b72f4
--- /dev/null
+++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.cpp
@@ -0,0 +1,10 @@
+#include <string>
+// CHECK: #include <string>
+// CHECK-NEXT: #include <memory>
+// CHECK-NEXT: #include "bar.h"
+#include <memory>
+#include "foo.h"
+#include "bar.h"
+
+void foo() {
+}
diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.yaml
new file mode 100644
index 000000000000000..172b5ee1f1ac5a1
--- /dev/null
+++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.yaml
@@ -0,0 +1,14 @@
+---
+MainSourceFile:  no.cpp
+Diagnostics:
+  - DiagnosticName:  test-header-format
+    DiagnosticMessage:
+      Message: Fix
+      FilePath: $(path)/no.cpp
+      FileOffset: 36
+      Replacements:
+        - FilePath:        $(path)/no.cpp
+          Offset:          36
+          Length:          17
+          ReplacementText: ""
+...
diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.cpp b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.cpp
new file mode 100644
index 000000000000000..20e6b90c39b6393
--- /dev/null
+++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.cpp
@@ -0,0 +1,10 @@
+#include <string>
+// CHECK: #include "bar.h"
+// CHECK-NEXT: #include <memory>
+// CHECK-NEXT: #include <string>
+#include <memory>
+#include "foo.h"
+#include "bar.h"
+
+void foo() {
+}
diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.yaml
new file mode 100644
index 000000000000000..723c4c5d5ceb391
--- /dev/null
+++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.yaml
@@ -0,0 +1,14 @@
+---
+MainSourceFile:  yes.cpp
+Diagnostics:
+  - DiagnosticName:  test-header-format
+    DiagnosticMessage:
+      Message: Fix
+      FilePath: $(path)/yes.cpp
+      FileOffset: 36
+      Replacements:
+        - FilePath:        $(path)/yes.cpp
+          Offset:          36
+          Length:          17
+          ReplacementText: ""
+...
diff --git a/clang-tools-extra/test/clang-apply-replacements/format-header.cpp b/clang-tools-extra/test/clang-apply-replacements/format-header.cpp
new file mode 100644
index 000000000000000..6a221c44b06a78f
--- /dev/null
+++ b/clang-tools-extra/test/clang-apply-replacements/format-header.cpp
@@ -0,0 +1,13 @@
+// RUN: mkdir -p %T/Inputs/format_header_yes
+// RUN: mkdir -p %T/Inputs/format_header_no
+//
+//
+// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/format_header/yes.cpp > %T/Inputs/format_header_yes/yes.cpp
+// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/format_header/no.cpp > %T/Inputs/format_header_no/no.cpp
+// RUN: sed "s#\$(path)#%/T/Inputs/format_header_yes#" %S/Inputs/format_header/yes.yaml > %T/Inputs/format_header_yes/yes.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/format_header_no#" %S/Inputs/format_header/no.yaml > %T/Inputs/format_header_no/no.yaml
+// RUN: clang-apply-replacements -format -style="{BasedOnStyle: llvm, SortIncludes: CaseSensitive}" %T/Inputs/format_header_yes
+// RUN: clang-apply-replacements %T/Inputs/format_header_no
+// RUN: FileCheck --strict-whitespace -input-file=%T/Inputs/format_header_yes/yes.cpp %S/Inputs/format_header/yes.cpp
+// RUN: FileCheck --strict-whitespace -input-file=%T/Inputs/format_header_no/no.cpp %S/Inputs/format_header/no.cpp
+//

``````````

</details>


https://github.com/llvm/llvm-project/pull/79466