Add format fix in git pre-commit hooks

[chenyukang]

I met this several times, I forget to run x fmt and push to remote, then get an format error and then rebase to fix it, it's trivial but it's a interruption.

If we add format fix in pre-commit, it won't happend again, it's a script like this one:
https://gist.github.com/folex/9496c457bcbbef36255a533389da740e

I tested in my local, seems works well.

Maybe we should add it to git hooks setup,
I'm not sure whether everyone like this behavior as default.

@jyn514

[chenyukang]

@rustbot claim

[jyn514]

We already run fmt --check in the pre-push hook, was that not running for you?

I don't think we should be running git add or otherwise be modifying commits automatically.

[chenyukang]

let me double check it.

[chenyukang]

fmt --check in pre-push running for me, but it only blocks pushing code not formatted well.
If I forget run x fmt, we need to fix it manully.

This git hook don't change other things, no changes on git logs, etc.
The git add will only run for those files which we want to add and commit, so it make no difference, except for formatting the code.

In this way, devloper don't care format anymore (no need to run x fmt manually).

For simple testing, put this code in .git/hooks/pre-commit:

#!/bin/bash

for file in $(git diff --name-only --staged); do
    echo "checking $file"
    rustfmt $file
    git add $file
done

I make some code with bad identation purposely, and if I commit the code, hook will fix the format:

cat@LAPTOP-V6U0QKD4:~/code/rust$ git diff .

diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs
index c316a4dd6b4..3621d9ef597 100644
--- a/compiler/rustc_parse/src/parser/diagnostics.rs
+++ b/compiler/rustc_parse/src/parser/diagnostics.rs
@@ -344,7 +344,8 @@ fn tokens_to_string(tokens: &[TokenType]) -> String {
             let mut i = tokens.iter();
             // This might be a sign we need a connect method on `Iterator`.
             let b = i.next().map_or_else(String::new, |t| t.to_string());
-            i.enumerate().fold(b, |mut b, (i, a)| {
+            i.enumerate().fold(b, |mut b, (i, a)|
+            {
                 if tokens.len() > 2 && i == tokens.len() - 2 {
                     b.push_str(", or ");
                 } else if tokens.len() == 2 && i == tokens.len() - 2 {
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index f6a6ed379a2..e5525faa010 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -2375,7 +2375,7 @@ fn error_on_extra_if(&mut self, cond: &P<Expr>) -> PResult<'a, ()> {
             let ExprKind::If(cond, ..) = &right.kind {
                     Err(self.sess.create_err(UnexpectedIfWithIf(binop_span.shrink_to_hi().to(cond.span.shrink_to_lo()))))
             } else {
-                Ok(())
+    Ok(())
             }
     }

See, we will work as don't care any fmt issue anymore.

cat@LAPTOP-V6U0QKD4:~/code/rust$ git commit -am"test"
checking compiler/rustc_parse/src/parser/diagnostics.rs
checking compiler/rustc_parse/src/parser/expr.rs
[yukang-debug-haha bceac7d5361] test
 2 files changed, 3 insertions(+), 2 deletions(-)
cat@LAPTOP-V6U0QKD4:~/code/rust$ git push
Running pre-push script 'python3 /home/cat/code/rust/x.py test tidy'
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.04s
Building stage0 tool tidy (x86_64-unknown-linux-gnu)
    Finished release [optimized + debuginfo] target(s) in 0.18s
tidy check
Checking which error codes lack tests...
* 633 error codes
* highest error code: E0791
Found 507 error codes
Found 0 error(s) in error codes
Done!
* 392 features
fmt check
Build completed successfully in 0:00:11
result: '0'
Enumerating objects: 15, done.
Counting objects: 100% (15/15), done.
Delta compression using up to 8 threads
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 647 bytes | 647.00 KiB/s, done.
Total 8 (delta 6), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (6/6), completed with 6 local objects.
To github.com:chenyukang/rust.git

[bjorn3]

Formatting all changed files is not correct. There are several directories that are not formatted. See

rust/rustfmt.toml

Lines 6 to 43 in 109cccb

	# by default we ignore everything in the repository
	# tidy only checks files which are not ignored, each entry follows gitignore style
	ignore = [
	"/build/",
	"/*-build/",
	"/build-*/",
	"/vendor/",
	# tests for now are not formatted, as they are sometimes pretty-printing constrained
	# (and generally rustfmt can move around comments in UI-testing incompatible ways)
	"src/test",
	# do not format submodules
	"library/backtrace",
	"library/portable-simd",
	"library/stdarch",
	"compiler/rustc_codegen_gcc",
	"src/doc/book",
	"src/doc/edition-guide",
	"src/doc/embedded-book",
	"src/doc/nomicon",
	"src/doc/reference",
	"src/doc/rust-by-example",
	"src/doc/rustc-dev-guide",
	"src/llvm-project",
	"src/tools/cargo",
	"src/tools/clippy",
	"src/tools/miri",
	"src/tools/rls",
	"src/tools/rust-analyzer",
	"src/tools/rustfmt",
	"src/tools/rust-installer",
	# these are ignored by a standard cargo fmt run
	"compiler/rustc_codegen_cranelift/y.rs", # running rustfmt breaks this file
	"compiler/rustc_codegen_cranelift/example",
	"compiler/rustc_codegen_cranelift/scripts",
	]

Also using plain rustfmt rather than the specific nightly used by rustc will cause incorrect formatting.

[chenyukang]

Formatting all changed files is not correct. There are several directories that are not formatted. See

rust/rustfmt.toml

Lines 6 to 43 in 109cccb

	# by default we ignore everything in the repository
	# tidy only checks files which are not ignored, each entry follows gitignore style
	ignore = [
	"/build/",
	"/*-build/",
	"/build-*/",
	"/vendor/",
	# tests for now are not formatted, as they are sometimes pretty-printing constrained
	# (and generally rustfmt can move around comments in UI-testing incompatible ways)
	"src/test",
	# do not format submodules
	"library/backtrace",
	"library/portable-simd",
	"library/stdarch",
	"compiler/rustc_codegen_gcc",
	"src/doc/book",
	"src/doc/edition-guide",
	"src/doc/embedded-book",
	"src/doc/nomicon",
	"src/doc/reference",
	"src/doc/rust-by-example",
	"src/doc/rustc-dev-guide",
	"src/llvm-project",
	"src/tools/cargo",
	"src/tools/clippy",
	"src/tools/miri",
	"src/tools/rls",
	"src/tools/rust-analyzer",
	"src/tools/rustfmt",
	"src/tools/rust-installer",
	# these are ignored by a standard cargo fmt run
	"compiler/rustc_codegen_cranelift/y.rs", # running rustfmt breaks this file
	"compiler/rustc_codegen_cranelift/example",
	"compiler/rustc_codegen_cranelift/scripts",
	]

Also using plain rustfmt rather than the specific nightly used by rustc will cause incorrect formatting.

yes, that's need to tweak if I want to implement it.
x fmt should do some special configurations..

[jyn514]

fmt --check in pre-push running for me, but it only blocks pushing code not formatted well.
If I forget run x fmt, we need to fix it manully.

Ok. I'm happy for you to use this hook yourself, but I don't think it belongs in the supported hook that x.py sets up.