Pre commit without cargo and other pre-PR improvements (#5146)

This tackles three problems:
* pre-commit was slow because it ran cargo commands
* Improve the clarity on what you need to run to get your PR pass on CI
(and make those fast)
* You had to compile and run `cargo dev generate-all` separately, which
was slow

The first change is to remove all cargo commands except running ruff
itself from pre-commit. With `cargo run --bin ruff` already compiled it
takes about 7s on my machine. It would make sense to also use the ruff
pre-commit action here even if we're then lagging a release behind for
checking ruff on ruff.

The contributing guide is now clear about what you need to run:

```shell
cargo clippy --workspace --all-targets --all-features -- -D warnings  # Linting...
RUFF_UPDATE_SCHEMA=1 cargo test  # Testing and updating ruff.schema.json
pre-commit run --all-files  # rust and python formatting, markdown and python linting, etc.
```

Example timings from my machine:

`cargo clippy --workspace --all-targets --all-features -- -D warnings`:
23s
`RUFF_UPDATE_SCHEMA=1 cargo test`: 2min (recompiling), 1min (no code
changes, this is mainly doc tests)
`pre-commit run --all-files`: 7s

The exact numbers don't matter so much as the approximate experience (6s
is easier to just wait than 1min, esp if you need to fix and rerun). The
biggest remaining block seems to be doc tests, i'm surprised i didn't
find any solution to speeding them up (nextest simply doesn't run them
at all). Also note that the formatter has it's own tests which are much
faster since they avoid linking ruff (`cargo test
ruff_python_formatter`).

The third change is to enable `cargo test` to update the schema. Similar
to `INSTA_UPDATE=always`, i've added `RUFF_UPDATE_SCHEMA=1` (name open
to bikeshedding), so `RUFF_UPDATE_SCHEMA=1 cargo test` updates the
schema, while `cargo test` still fails as expected if the repo isn't
up-to-date.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>

.pre-commit-config.yaml

@@ -38,29 +38,19 @@ repos:
         name: cargo fmt
         entry: cargo fmt --
         language: system
-        types: [rust]
-      - id: clippy
-        name: clippy
-        entry: cargo clippy --workspace --all-targets --all-features -- -D warnings
-        language: system
-        pass_filenames: false
+        types: [ rust ]
+        pass_filenames: false # This makes it a lot faster
       - id: ruff
         name: ruff
-        entry: cargo run -p ruff_cli -- check --no-cache --force-exclude --fix --exit-non-zero-on-fix
+        entry: cargo run --bin ruff -- check --no-cache --force-exclude --fix --exit-non-zero-on-fix
         language: system
-        types_or: [python, pyi]
+        types_or: [ python, pyi ]
         require_serial: true
         exclude: |
           (?x)^(
             crates/ruff/resources/.*|
             crates/ruff_python_formatter/resources/.*
           )$
-      - id: dev-generate-all
-        name: dev-generate-all
-        entry: cargo dev generate-all
-        language: system
-        pass_filenames: false
-        exclude: target
 
   # Black
   - repo: https://github.com/psf/black
@@ -69,4 +59,4 @@ repos:
       - id: black
 
 ci:
-  skip: [cargo-fmt, clippy, dev-generate-all]
+  skip: [ cargo-fmt, dev-generate-all ]

CONTRIBUTING.md

@@ -45,6 +45,12 @@ You'll also need [Insta](https://insta.rs/docs/) to update snapshot tests:
 cargo install cargo-insta
 ```
 
+and pre-commit to run some validation checks:
+
+```shell
+pipx install pre-commit  # or `pip install pre-commit` if you have a virtualenv
+```
+
 ### Development
 
 After cloning the repository, run Ruff locally with:
@@ -57,9 +63,9 @@ Prior to opening a pull request, ensure that your code has been auto-formatted,
 and that it passes both the lint and test validation checks:
 
 ```shell
-cargo fmt  # Auto-formatting...
-cargo test  # Testing...
-cargo clippy --workspace --all-targets --all-features -- -D warnings  # Linting...
+cargo clippy --workspace --all-targets --all-features -- -D warnings  # Rust linting
+RUFF_UPDATE_SCHEMA=1 cargo test  # Rust testing and updating ruff.schema.json
+pre-commit run --all-files --show-diff-on-failure  # Rust and Python formatting, Markdown and Python linting, etc.
 ```
 
 These checks will run on GitHub Actions when you open your Pull Request, but running them locally
@@ -72,13 +78,6 @@ after running `cargo test` like so:
 cargo insta review
 ```
 
-If you have `pre-commit` [installed](https://pre-commit.com/#installation) then you can use it to
-assist with formatting and linting. The following command will run the `pre-commit` hooks:
-
-```shell
-pre-commit run --all-files
-```
-
 Your Pull Request will be reviewed by a maintainer, which may involve a few rounds of iteration
 prior to merging.
 

crates/ruff_dev/src/generate_json_schema.rs

@@ -32,15 +32,20 @@ pub(crate) fn main(args: &Args) -> Result<()> {
         Mode::Check => {
             let current = fs::read_to_string(schema_path)?;
             if current == schema_string {
-                println!("up-to-date: {filename}");
+                println!("Up-to-date: {filename}");
             } else {
                 let comparison = StrComparison::new(&current, &schema_string);
                 bail!("{filename} changed, please run `{REGENERATE_ALL_COMMAND}`:\n{comparison}");
             }
         }
         Mode::Write => {
-            let file = schema_path;
-            fs::write(file, schema_string.as_bytes())?;
+            let current = fs::read_to_string(&schema_path)?;
+            if current == schema_string {
+                println!("Up-to-date: {filename}");
+            } else {
+                println!("Updating: {filename}");
+                fs::write(schema_path, schema_string.as_bytes())?;
+            }
         }
     }
 
@@ -50,6 +55,7 @@ pub(crate) fn main(args: &Args) -> Result<()> {
 #[cfg(test)]
 mod test {
     use anyhow::Result;
+    use std::env;
 
     use crate::generate_all::Mode;
 
@@ -57,6 +63,11 @@ mod test {
 
     #[test]
     fn test_generate_json_schema() -> Result<()> {
-        main(&Args { mode: Mode::Check })
+        let mode = if env::var("RUFF_UPDATE_SCHEMA").as_deref() == Ok("1") {
+            Mode::Write
+        } else {
+            Mode::Check
+        };
+        main(&Args { mode })
     }
 }