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

[clang-format] Fix a serious bug in git clang-format -f #102629

Merged
merged 4 commits into from
Aug 10, 2024
Merged

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Aug 9, 2024

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 #102459.

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.
@llvmbot
Copy link
Member

llvmbot commented Aug 9, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

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 #102459.


Full diff: https://github.com/llvm/llvm-project/pull/102629.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 (+10-2)
diff --git a/clang/test/Format/list-ignored.cpp b/clang/test/Format/list-ignored.cpp
new file mode 100644
index 00000000000000..91f2b5175d6395
--- /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 -match-full-lines
+// 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 -match-full-lines
+// 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 -match-full-lines
+// 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 -match-full-lines
+// 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 -match-full-lines
+// CHECK6-NOT: level1/levle2/foo.c
+// CHECK6: foo.cc
+// CHECK6-NEXT: level1/bar.cc
+// CHECK6-NEXT: level1/baz.c
+// CHECK6-NEXT: level1/level2/foo.js
+
+// RUN: rm .clang-format-ignore
+// RUN: clang-format -list-ignored *.cc level1/*.c* level1/level2/foo.* \
+// RUN:   | FileCheck %s -check-prefix=CHECK7 -match-full-lines
+// CHECK7-NOT: foo.cc
+// CHECK7-NOT: level1/bar.cc
+// CHECK7-NOT: level1/baz.c
+// CHECK7-NOT: level1/level2/foo.c
+// CHECK7: level1/level2/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 6582d73eae40ea..50dd5345a6db33 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..8f7eb38d72e738 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,13 @@ 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()).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')

owenca added 3 commits August 9, 2024 08:30
Changes the `"\n"` to `'\n'`.
Handles the case when `clang-format -list-ignored` outputs nothing.
Handle Windows.
@owenca owenca merged commit 986bc3d into llvm:main Aug 10, 2024
8 checks passed
@owenca owenca deleted the 102459 branch August 10, 2024 20:31
@owenca
Copy link
Contributor Author

owenca commented Aug 10, 2024

/cherry-pick 986bc3d

@owenca owenca added this to the LLVM 19.X Release milestone Aug 10, 2024
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 10, 2024
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)
@llvmbot
Copy link
Member

llvmbot commented Aug 10, 2024

/pull-request #102770

@stefanpantic-pdftools
Copy link

FYI: this breaks the tool:

git clang-format --binary /usr/bin/clang-format --extensions $INPUT_EXTENSIONS --diff origin/$INPUT_TARGET_BRANCH origin/$INPUT_SOURCE_BRANCH
`/usr/bin/clang-format -list-ignored` returned 1
clang-format: Unknown command line argument '-list-ignored'.  Try: '/usr/bin/clang-format --help'
clang-format: Did you mean '--lines'?

@owenca
Copy link
Contributor Author

owenca commented Aug 12, 2024

You need to use the clang-format built from this patch, which adds the new clang-format -list-ignored option.

@stefanpantic-pdftools
Copy link

Gotcha, thanks!

@llvm llvm deleted a comment from llvm-ci Aug 12, 2024
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 13, 2024
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)
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.

git-clang-format clears files listed in .clang-format-ignore
4 participants