Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release/19.x: [clang-format] Fix a serious bug in git clang-format -f (#102629) #102770

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 10, 2024

Backport 986bc3d

Requested by: @owenca

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Aug 10, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 10, 2024

@HazardyKnusperkeks What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Aug 10, 2024

@llvm/pr-subscribers-clang-format

Author: None (llvmbot)

Changes

Backport 986bc3d

Requested by: @owenca


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

3 Files Affected:

  • (added) clang/test/Format/list-ignored.cpp (+60)
  • (modified) clang/tools/clang-format/ClangFormat.cpp (+11-1)
  • (modified) clang/tools/clang-format/git-clang-format (+13-2)
diff --git a/clang/test/Format/list-ignored.cpp b/clang/test/Format/list-ignored.cpp
new file mode 100644
index 00000000000000..6e65a68a6f9968
--- /dev/null
+++ b/clang/test/Format/list-ignored.cpp
@@ -0,0 +1,60 @@
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/level1/level2
+
+// RUN: cd %t.dir
+// RUN: echo "*" > .clang-format-ignore
+// RUN: echo "level*/*.c*" >> .clang-format-ignore
+// RUN: echo "*/*2/foo.*" >> .clang-format-ignore
+
+// RUN: touch foo.cc
+// RUN: clang-format -list-ignored .clang-format-ignore foo.cc \
+// RUN:   | FileCheck %s
+// CHECK: .clang-format-ignore
+// CHECK-NEXT: foo.cc
+
+// RUN: cd level1
+// RUN: touch bar.cc baz.c
+// RUN: clang-format -list-ignored bar.cc baz.c \
+// RUN:   | FileCheck %s -check-prefix=CHECK2
+// CHECK2: bar.cc
+// CHECK2-NEXT: baz.c
+
+// RUN: cd level2
+// RUN: touch foo.c foo.js
+// RUN: clang-format -list-ignored foo.c foo.js \
+// RUN:   | FileCheck %s -check-prefix=CHECK3
+// CHECK3: foo.c
+// CHECK3-NEXT: foo.js
+
+// RUN: touch .clang-format-ignore
+// RUN: clang-format -list-ignored foo.c foo.js \
+// RUN:   | FileCheck %s -allow-empty -check-prefix=CHECK4
+// CHECK4-NOT: foo.c
+// CHECK4-NOT: foo.js
+
+// RUN: echo "*.js" > .clang-format-ignore
+// RUN: clang-format -list-ignored foo.c foo.js \
+// RUN:   | FileCheck %s -check-prefix=CHECK5
+// CHECK5-NOT: foo.c
+// CHECK5: foo.js
+
+// RUN: cd ../..
+// RUN: clang-format -list-ignored *.cc level1/*.c* level1/level2/foo.* \
+// RUN:   | FileCheck %s -check-prefix=CHECK6
+// CHECK6: foo.cc
+// CHECK6-NEXT: bar.cc
+// CHECK6-NEXT: baz.c
+// CHECK6-NOT: foo.c
+// CHECK6-NEXT: foo.js
+
+// RUN: rm .clang-format-ignore
+// RUN: clang-format -list-ignored *.cc level1/*.c* level1/level2/foo.* \
+// RUN:   | FileCheck %s -check-prefix=CHECK7
+// CHECK7-NOT: foo.cc
+// CHECK7-NOT: bar.cc
+// CHECK7-NOT: baz.c
+// CHECK7-NOT: foo.c
+// CHECK7: foo.js
+
+// RUN: cd ..
+// RUN: rm -r %t.dir
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index 6cba1267f3b0db..c4b6209a71a88f 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -210,6 +210,10 @@ static cl::opt<bool> FailOnIncompleteFormat(
     cl::desc("If set, fail with exit code 1 on incomplete format."),
     cl::init(false), cl::cat(ClangFormatCategory));
 
+static cl::opt<bool> ListIgnored("list-ignored",
+                                 cl::desc("List ignored files."),
+                                 cl::cat(ClangFormatCategory), cl::Hidden);
+
 namespace clang {
 namespace format {
 
@@ -715,7 +719,13 @@ int main(int argc, const char **argv) {
   unsigned FileNo = 1;
   bool Error = false;
   for (const auto &FileName : FileNames) {
-    if (isIgnored(FileName))
+    const bool Ignored = isIgnored(FileName);
+    if (ListIgnored) {
+      if (Ignored)
+        outs() << FileName << '\n';
+      continue;
+    }
+    if (Ignored)
       continue;
     if (Verbose) {
       errs() << "Formatting [" << FileNo++ << "/" << FileNames.size() << "] "
diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format
index d33fd478d77fd9..714ba8a6e77d51 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -173,11 +173,12 @@ def main():
   # those files.
   cd_to_toplevel()
   filter_symlinks(changed_lines)
+  filter_ignored_files(changed_lines, binary=opts.binary)
   if opts.verbose >= 1:
     ignored_files.difference_update(changed_lines)
     if ignored_files:
-      print(
-        'Ignoring changes in the following files (wrong extension or symlink):')
+      print('Ignoring the following files (wrong extension, symlink, or '
+            'ignored by clang-format):')
       for filename in ignored_files:
         print('    %s' % filename)
     if changed_lines:
@@ -399,6 +400,16 @@ def filter_symlinks(dictionary):
       del dictionary[filename]
 
 
+def filter_ignored_files(dictionary, binary):
+  """Delete every key in `dictionary` that is ignored by clang-format."""
+  ignored_files = run(binary, '-list-ignored', *dictionary.keys())
+  if not ignored_files:
+    return
+  ignored_files = ignored_files.split('\n')
+  for filename in ignored_files:
+    del dictionary[filename]
+
+
 def cd_to_toplevel():
   """Change to the top level of the git repository."""
   toplevel = run('git', 'rev-parse', '--show-toplevel')

With the --force (or -f) option, git-clang-format wipes out input files
excluded by a .clang-format-ignore file if they have unstaged changes.

This patch adds a hidden clang-format option --list-ignored that lists
such excluded files for git-clang-format to filter out.

Fixes llvm#102459.

(cherry picked from commit 986bc3d)
@tru tru merged commit efdd0e9 into llvm:release/19.x Aug 13, 2024
8 of 10 checks passed
Copy link

@owenca (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants