"cargo new" should add pre-commit hooks for autoformatter and linter
Problem
- Usage of autoformatter and linter is a really good practice and, for not depend on IDE, good to have them in pre-commit hooks (of course it can also be some rust implementation (if any)) for block commiting changes which has issues.
- Rust/cargo has rustfmt and clippy.
-
cargo neweven does initialization of new git repo, has all tools, but doesn't configure pre-commit hooks, but why to not add them as it clearly good practice?
Proposed Solution
cargo new should depends on pre-commit or something similar and add config (.pre-commit-config.yaml for pre-commit).
Notes
For rustfmt I found example here:
repos:
- repo: local
hooks:
- id: rustfmt
name: rustfmt
description: Check if all files follow the rustfmt style
entry: cargo fmt --all -- --check --color always
language: system
pass_filenames: false
another example, plus example for clippy can be found here:
- repo: local
hooks:
- id: rust-linting
name: Rust linting
description: Run cargo fmt on files included in the commit. rustfmt should be installed before-hand.
entry: cargo fmt --all --
pass_filenames: true
types: [file, rust]
language: system
- id: rust-clippy
name: Rust clippy
description: Run cargo clippy on files included in the commit. clippy should be installed before-hand.
entry: cargo clippy --all-targets --all-features -- -Dclippy::all
pass_filenames: false
types: [file, rust]
language: system
A pre-commit hook is fairly opinionated choice
- It is useless unless the user has the commit hook program installed
- It is useless unless the hooks get registered (see above)
- iirc commit hooks are not composable unless you completely buy into one specific pre-commit hook tool
Also, for myself, one of the most frustrating thing about project scaffolding tools is all of the content you have to delete. I'd rather not be generating this content unless its going to be used or else we are pushing for people to delete it.
imo this seems better suited to be solved by either
- #5420
- #5151
- About pre-commit: IMO it's just kind of standard tool for git hooks, so, shouldn't be big issue to install it as a cargo dependency and run
pre-commit install, but I think git hooks can be added without it if any concerns about dependency. - Agree that in many cases default setup is not that useful, but it's bigger topic about defaults in general. At least for now
cargo new's defaults looks good for new starters and it's good. So, if defaults are for people who starting learn Rust might be good to show all best practices.
But it just an idea, thank you for nice tooling!
From the previous discussion https://github.com/rust-lang/cargo/issues/12102#issuecomment-1540686779, the team generally leans toward adding extra VCS support via some extension points in Cargo, to lighten the maintenance burden. Adding new features for existing VCS support falls into the same category as that, I think.
Rustup's minimal profile does not include rustfmt or clippy. I do not really believe Cargo should add anything that assumes or significantly varies its user experience based on whether it ships with tools that do not appear in the minimal profile.
...My opinion is admittedly also informed by the fact that I run git commit --amend about 4 times per commit and rebase every PR about twice before I push it, and clippy and rustfmt can contain algorithms with a performance worse than O(n) (partly because they use the Rust compiler, which has such algorithms already).
whether it ships with tools that do not appear in the minimal profile.
That's a pretty fair point! Cargo also has no knowledge about rustup profiles. At least Cargo needs extra executable probes to find their existence.