Skip to content

Conversation

@BaumiCoder
Copy link

@BaumiCoder BaumiCoder commented Sep 21, 2025

In Rust there is a default formatter, called rustfmt, and a default linter, called clippy. This Pull Request adds the names (with and without leading dot) of their config files (see commit message for links to the respective documentations).
Furthermore, the single file name for cargo-deny, a linter focused on cargo / Rust dependencies.

(Useful, when running them with pre-commit and only want to run, if relevant files have changed, i.e., rust files, Cargo.toml or the config file in the repository. Example pre-commit config for a Rust project here)

@asottile
Copy link
Member

I don't see how this can be useful? why wouldn't this just use files to match these? they're already matched by toml without special conventions

@BaumiCoder
Copy link
Author

With that changes, for example I would use this for rustfmt:

types_or: [cargo, rust, rustfmt]

I am not sure, what your alternative approach with the same filtering would be. For example this would not match anymore the *.rs files, because there is an "and" between files and types_or, isn't it?

files: "\\.?rustfmt\\.toml"
types_or: [cargo, rust, toml]

So everything, needs to be defined with files in this way,

files: "(\\.?rustfmt\\.toml|Cargo.toml|.+\\.rs)"

which is not that readable.

Only using types_or would also include other TOML files, which are not relevant for rustfmt

types_or: [cargo, rust, toml]

@asottile
Copy link
Member

that doesn't really make sense, you would just use types: [rust] such that pre-commit passes just the rust files to the hook

@BaumiCoder
Copy link
Author

I call it on the whole repository:

      - id: rustfmt
        name: rustfmt
        description: The Rust default formatter (see .rustfmt.toml for config)
        # Run via cargo to use edition from Cargo.toml
        entry: cargo fmt --check --verbose
        language: rust
        types_or: [cargo, rust]
        pass_filenames: false

Running rustfmt via cargo allows getting the edition property from the Cargo.toml. Furthermore, I want to run it also when it config changes (i.e., the rustfmt.toml) to avoid that a config change makes the formatting invalid in respect to the new config.

Similar is for the two linter, which are also called on the whole repository and config changes are relevant.

      - id: cargo-deny
        name: cargo-deny
        description: A Linter which checks Rust dependencies (e.g., regarding licenses)
        entry: cargo deny check
        language: rust
        types_or: [cargo, cargo-lock]
        pass_filenames: false
        additional_dependencies: [cli:cargo-deny:0.18.*]
      - id: clippy
        name: clippy
        description: The Rust default linter
        entry: cargo clippy
        language: rust
        types_or: [cargo, rust]
        pass_filenames: false
        stages: [pre-push]

@asottile
Copy link
Member

I call it on the whole repository

yeah I'd say you're holding it wrong then. part of the point of pre-commit is to only run on the things that change

@BaumiCoder
Copy link
Author

Okay, so is the right way to run rustfmt only on the changed *.rs files and run the on the whole directory only if the config file changes (with two separate hooks)?

Furthermore, cargo-deny only process the config files (from cargo and from deny) as it only considers the dependencies of a project. For clippy as a normal linter, I would expect a support for processing single files, but that seems to be missing (see Issue).

@BaumiCoder
Copy link
Author

I reworked my pre-commit config, but I still see the need for these new types, even for rustfmt. These would allow using types_or instead of the files in these lines:

@asottile
Copy link
Member

no I wouldn't bother with a "run everything" hook. just do that manually and you should be running everything in ci already anyway.

@BaumiCoder
Copy link
Author

In my CI (at Codeberg) I plan to use pre-commit run --all-files -v --hook-stage manual. This avoids a separate config for linters (and their versions) as pre-commit handles the installation of all necessary linters for the CI.

@asottile
Copy link
Member

indeed so you don't really need this:

and run the on the whole directory only if the config file changes (with two separate hooks)?

@BaumiCoder
Copy link
Author

Okay, I understand your point of view now. The CI is enough to ensure that the project and its config fits together and the pre-commit checks for this is not necessary, and you do not need to spend computation time on them. The other perspective is to avoid CI fails as much as possible with pre-commit checks to fix issues earlier. Something like this is always a trade-off.

@BaumiCoder BaumiCoder closed this Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants