From cea9e34942e8295fccd2c82c21a10ebbbef83791 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 6 Nov 2022 14:31:16 -0500 Subject: [PATCH] Update CONTRIBUTING.md (#623) --- CONTRIBUTING.md | 56 ++++++++++------------ ruff_dev/src/generate_check_code_prefix.rs | 2 +- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2420a69004..8af18c9259 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,17 +1,17 @@ -# Contributing to ruff +# Contributing to Ruff -Welcome! We're happy to have you here. Thank you in advance for your contribution to ruff. +Welcome! We're happy to have you here. Thank you in advance for your contribution to Ruff. ## The basics -ruff welcomes contributions in the form of Pull Requests. For small changes (e.g., bug fixes), feel +Ruff welcomes contributions in the form of Pull Requests. For small changes (e.g., bug fixes), feel free to submit a PR. For larger changes (e.g., new lint rules, new functionality, new configuration options), consider submitting an [Issue](https://github.com/charliermarsh/ruff/issues) outlining your proposed change. ### Prerequisites -ruff is written in Rust. You'll need to install the +Ruff is written in Rust. You'll need to install the [Rust toolchain](https://www.rust-lang.org/tools/install) for development. You'll also need [Insta](https://insta.rs/docs/) to update snapshot tests: @@ -22,7 +22,7 @@ cargo install cargo-insta ### Development -After cloning the repository, run ruff locally with: +After cloning the repository, run Ruff locally with: ```shell cargo run resources/test/fixtures --no-cache @@ -32,9 +32,9 @@ Prior to opening a pull request, ensure that your code has been auto-formatted, both the lint and test validation checks: ```shell -cargo fmt # Auto-formatting... -cargo clippy # Linting... -cargo test # Testing... +cargo +nightly fmt --all # Auto-formatting... +cargo +nightly clippy --all # Linting... +cargo +nightly test --all # Testing... ``` These checks will run on GitHub Actions when you open your Pull Request, but running them locally @@ -45,12 +45,13 @@ prior to merging. ### Example: Adding a new lint rule -There are three phases to adding a new lint rule: +There are four phases to adding a new lint rule: 1. Define the rule in `src/checks.rs`. -2. Define the _logic_ for triggering the rule in `src/check_ast.rs` (for AST-based checks) - or `src/check_lines.rs` (for text-based checks). +2. Define the _logic_ for triggering the rule in `src/check_ast.rs` (for AST-based checks), + `src/check_tokens.rs` (for token-based checks), or `src/check_lines.rs` (for text-based checks). 3. Add a test fixture. +4. Update the generated files (documentation and generated code). To define the rule, open up `src/checks.rs`. You'll need to define both a `CheckCode` and `CheckKind`. As an example, you can grep for `E402` and `ModuleImportNotAtTopOfFile`, and follow the @@ -59,37 +60,32 @@ pattern implemented therein. To trigger the rule, you'll likely want to augment the logic in `src/check_ast.rs`, which defines the Python AST visitor, responsible for iterating over the abstract syntax tree and collecting lint-rule violations as it goes. If you need to inspect the AST, you can run `cargo dev print-ast` -with a python file. Grep for the `Check::new` invocations to understand how other, similar rules +with a Python file. Grep for the `Check::new` invocations to understand how other, similar rules are implemented. To add a test fixture, create a file under `resources/test/fixtures`, named to match the `CheckCode` you defined earlier (e.g., `E402.py`). This file should contain a variety of violations and -non-violations designed to evaluate and demonstrate the behavior of your lint rule. Run ruff locally -with (e.g.) `cargo run resources/test/fixtures/E402.py`. Once you're satisfied with the output, -codify the behavior as a snapshot test by adding a new function to the `mod tests` section of -`src/linter.rs`, like so: +non-violations designed to evaluate and demonstrate the behavior of your lint rule. Run Ruff locally +with (e.g.) `cargo run resources/test/fixtures/E402.py --no-cache`. Once you're satisfied with the +output, codify the behavior as a snapshot test by adding a new `testcase` macro to the `mod tests` +section of `src/linter.rs`, like so: ```rust -#[test] -fn e402() -> Result<()> { - let mut checks = check_path( - Path::new("./resources/test/fixtures/E402.py"), - &settings::Settings::for_rule(CheckCode::E402), - &fixer::Mode::Generate, - )?; - checks.sort_by_key(|check| check.location); - insta::assert_yaml_snapshot!(checks); - Ok(()) -} +#[test_case(CheckCode::A001, Path::new("A001.py"); "A001")] +... ``` Then, run `cargo test`. Your test will fail, but you'll be prompted to follow-up with `cargo insta review`. Accept the generated snapshot, then commit the snapshot file alongside the rest of your changes. +Finally, to update the documentation, run `cargo dev generate-rules-table` from the repo root. To +update the generated prefix map, run `cargo dev generate-check-code-prefix`. Both of these commands +should be run whenever a new check is added to the codebase. + ### Example: Adding a new configuration option -ruff's user-facing settings live in two places: first, the command-line options defined with +Ruff's user-facing settings live in two places: first, the command-line options defined with [clap](https://docs.rs/clap/latest/clap/) via the `Cli` struct in `src/main.rs`; and second, the `Config` struct defined `src/pyproject.rs`, which is responsible for extracting user-defined settings from a `pyproject.toml` file. @@ -104,9 +100,9 @@ acceptable unused variables (e.g., `_`). ## Release process -As of now, ruff has an ad hoc release process: releases are cut with high frequency via GitHub +As of now, Ruff has an ad hoc release process: releases are cut with high frequency via GitHub Actions, which automatically generates the appropriate wheels across architectures and publishes them to [PyPI](https://pypi.org/project/ruff/). -ruff follows the [semver](https://semver.org/) versioning standard. However, as pre-1.0 software, +Ruff follows the [semver](https://semver.org/) versioning standard. However, as pre-1.0 software, even patch releases may contain [non-backwards-compatible changes](https://semver.org/#spec-item-4). diff --git a/ruff_dev/src/generate_check_code_prefix.rs b/ruff_dev/src/generate_check_code_prefix.rs index 345e321eb0..fd8a8924c8 100644 --- a/ruff_dev/src/generate_check_code_prefix.rs +++ b/ruff_dev/src/generate_check_code_prefix.rs @@ -11,7 +11,7 @@ use itertools::Itertools; use ruff::checks::CheckCode; use strum::IntoEnumIterator; -const FILE: &str = "../src/checks_gen.rs"; +const FILE: &str = "src/checks_gen.rs"; #[derive(Parser)] #[command(author, version, about, long_about = None)]