FeryET / pre-commit-rust

Rust hooks for pre-commit
8 stars 4 forks source link

Also run hooks when only Cargo.toml and/or Cargo.lock has changed #1

Open tyilo opened 5 months ago

tyilo commented 5 months ago

Also see https://github.com/doublify/pre-commit-rust/issues/33


Both the cargo-check and clippy hooks should also be run if only Cargo.toml and/or Cargo.lock has been changed.

The following shows the problem:

$ cd /tmp
$ cargo new test-pre-commit
$ cd test-pre-commit
$ cat > .pre-commit-config.yaml
repos:
- repo: https://github.com/FeryET/pre-commit-rust
  rev: v1.1.0
  hooks:
    - id: fmt
    - id: cargo-check
^D
$ pre-commit install
$ git add .
$ git commit -m 'init'
fmt......................................................................Passed
cargo check..............................................................Passed
...
$ cargo add libc
$ cat > src/main.rs
fn main() {
    let r = unsafe { libc::rand() };
    dbg!(r);
}
^D
$ git add .
$ git commit -m '2nd'
fmt......................................................................Passed
cargo check..............................................................Passed
...
$ cargo rm libc
$ git add .
$ git commit -m '3rd'
fmt..................................................(no files to check)Skipped
cargo check..........................................(no files to check)Skipped

In the 3rd commit we have removed libc so that the code no longer compiles. However the cargo-check step is skipped, so this is not caught :(

Running pre-commit run --all-files manually results in:

fmt......................................................................Passed
cargo check..............................................................Failed
- hook id: cargo-check
- exit code: 101

    Checking test-pre-commit v0.1.0 (/tmp/test-pre-commit)
error[E0433]: failed to resolve: use of undeclared crate or module `libc`
 --> src/main.rs:2:22
  |
2 |     let r = unsafe { libc::rand() };
  |                      ^^^^ use of undeclared crate or module `libc`

For more information about this error, try `rustc --explain E0433`.
error: could not compile `test-pre-commit` (bin "test-pre-commit") due to 1 previous error
FeryET commented 2 months ago

I will look into this. @tyilo

FeryET commented 2 months ago

@tyilo I have tweaked the hook configs and added cargo.toml and cargo.lock to the cargo-check hook. As of 'v1.1.1` therer should be no issue regarding this. Please check it out and post an update here if you had any problems.

Re clippy I don't see the reason why it should be run when Cargo.toml changes. If you could elaborate I will work on that too.

tyilo commented 1 month ago

@tyilo I have tweaked the hook configs and added cargo.toml and cargo.lock to the cargo-check hook. As of 'v1.1.1` therer should be no issue regarding this. Please check it out and post an update here if you had any problems.

Doesn't seem to work:

$ cd /tmp
$ cargo new test-pre-commit
$ cd test-pre-commit
$ cat > .pre-commit-config.yaml
repos:
- repo: https://github.com/FeryET/pre-commit-rust
  rev: v1.1.1
  hooks:
    - id: fmt
    - id: cargo-check
^D
$ pre-commit install
$ git add .
$ git commit -m 'init'
fmt......................................................................Failed
- hook id: fmt
- exit code: 1

error: expected item, found `[`
 --> /tmp/test-pre-commit/Cargo.toml:1:1
  |
1 | [package]
  | ^ expected item
  |
  = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>

cargo check..............................................................Passed

Re clippy I don't see the reason why it should be run when Cargo.toml changes. If you could elaborate I will work on that too.

I only use the fmt and clippy hooks as clippy also shows compiler warnings/errors, so adding cargo-check to my list of hooks would be redundant.

Thus the clippy hook should also run when Cargo.toml/Cargo.lock changes.