https://github.com/dmpolukhin created https://github.com/llvm/llvm-project/pull/79466
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 >From 67a266e7bc5682d5f674c0424858ba86f7c9a192 Mon Sep 17 00:00:00 2001 From: Kugan <34810920+kug...@users.noreply.github.com> Date: Tue, 31 Oct 2023 12:12:10 +0000 Subject: [PATCH] Apply format only if --format is specified 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 --- .../tool/ClangApplyReplacementsMain.cpp | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 7 +++++++ .../Inputs/format_header/no.cpp | 10 ++++++++++ .../Inputs/format_header/no.yaml | 14 ++++++++++++++ .../Inputs/format_header/yes.cpp | 10 ++++++++++ .../Inputs/format_header/yes.yaml | 14 ++++++++++++++ .../clang-apply-replacements/format-header.cpp | 13 +++++++++++++ 7 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.cpp create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/no.yaml create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.cpp create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/format_header/yes.yaml create mode 100644 clang-tools-extra/test/clang-apply-replacements/format-header.cpp 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 +// _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits