tweag / pirouette

Language-generic workbench for building static analysis
MIT License
47 stars 2 forks source link

Nix-managed pre-commit hooks #172

Closed Niols closed 1 year ago

Niols commented 1 year ago

This PR introduces Nix-managed pre-commit hooks using pre-commit-hooks.nix. I think this dependency is not going to be a problem as it is maintained by Cachix (the company).

It brings much simpler and saner pre-commit hooks (the current one are driving me crazy). Adding pre-commit hooks is simply a matter of adding eg. ormolu.enable = true in the list of hooks in the flake.nix file. After that, one gets pre-commit hooks that do not silently change files but instead fail and change the files on the disk, allowing you to review or craft again a clean commit. I tested this PR with its own code, with purposefully badly written Nix in flake.nix initially:

$ git commit -m 'Add pre-commit check for nixfmt'
files to format 
nixfmt...................................................................Failed
- hook id: nixfmt
- files were modified by this hook

$ git add .

$ git commit -m 'Add pre-commit check for nixfmt'
files to format 
nixfmt...................................................................Passed
[main c8013a2] Add pre-commit check for nixfmt
 2 files changed, 9 insertions(+)
 create mode 120000 .pre-commit-config.yaml

The pre-commit hooks are transparently added to anyone using a nix develop. There is also a Flake check target that can be used on one's machine or in CI to ensure that pre-commit hooks were run by everyone.

This PR is still draft as I still have to add a bunch of missing hooks.

Niols commented 1 year ago

3aeb634db5da05e23a5f230de36809fdb61b93e2 re-adds an Ormolu pre-commit hook. I gave a test to the complicated case where one only stages part of a file that is poorly formatted. I am quite convinced by the result: then, the pre-commit hook fails but changes nothing. If full files are staged but poorly formatted, then the pre-commit hook fails but does format the files in place. If the files are properly formatted (including unstaged parts), the pre-commit hook succeeds. Neat!

Niols commented 1 year ago

I reverted adding the hlint hook because it involves cleaning up a lot of Haskell files and I don't think that belongs in this PR. cf https://github.com/tweag/pirouette/issues/173

Niols commented 1 year ago

Other hooks could be added, such as:

Feel free to play with that in another PR; this one was mostly here to set up the environment and demonstrate what it could do.

Niols commented 1 year ago

The last commits show that CI does catch errors. I think we are good to go. @gabrielhdt, if you wanna take a look, it's ready for you!