From 51f2a6f8b6eea9ebefddff39e87a1ca16c59827c Mon Sep 17 00:00:00 2001 From: flip1995 <hello@philkrones.com> Date: Wed, 22 Jul 2020 16:39:58 +0200 Subject: [PATCH 1/3] Add documentation for basic Clippy hacking --- CONTRIBUTING.md | 22 ++++----- doc/adding_lints.md | 7 +-- doc/basics.md | 106 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 16 deletions(-) create mode 100644 doc/basics.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 69a734e4ee4..dfc5cc077c3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,7 +32,7 @@ High level approach: 1. Find something to fix/improve 2. Change code (likely some file in `clippy_lints/src/`) -3. Follow the instructions in the [docs for writing lints](doc/adding_lints.md) such as running the `setup-toolchain.sh` script +3. Follow the instructions in the [Basics docs](doc/basics.md) such as running the `setup-toolchain.sh` script 4. Run `cargo test` in the root directory and wiggle code until it passes 5. Open a PR (also can be done after 2. if you run into problems) @@ -95,16 +95,16 @@ quick read. ## Getting code-completion for rustc internals to work -Unfortunately, [`rust-analyzer`][ra_homepage] does not (yet?) understand how Clippy uses compiler-internals -using `extern crate` and it also needs to be able to read the source files of the rustc-compiler which are not -available via a `rustup` component at the time of writing. -To work around this, you need to have a copy of the [rustc-repo][rustc_repo] available which can be obtained via -`git clone https://github.com/rust-lang/rust/`. -Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies -which rust-analyzer will be able to understand. -Run `cargo dev ra-setup --repo-path <repo-path>` where `<repo-path>` is an absolute path to the rustc repo -you just cloned. -The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to +Unfortunately, [`rust-analyzer`][ra_homepage] does not (yet?) understand how Clippy uses compiler-internals +using `extern crate` and it also needs to be able to read the source files of the rustc-compiler which are not +available via a `rustup` component at the time of writing. +To work around this, you need to have a copy of the [rustc-repo][rustc_repo] available which can be obtained via +`git clone https://github.com/rust-lang/rust/`. +Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies +which rust-analyzer will be able to understand. +Run `cargo dev ra-setup --repo-path <repo-path>` where `<repo-path>` is an absolute path to the rustc repo +you just cloned. +The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to Clippys `Cargo.toml`s and should allow rust-analyzer to understand most of the types that Clippy uses. Just make sure to remove the dependencies again before finally making a pull request! diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 8092be277cc..d5f4f5d5659 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -27,10 +27,7 @@ because that's clearly a non-descriptive name. ## Setup -When working on Clippy, you will need the current git master version of rustc, -which can change rapidly. Make sure you're working near rust-clippy's master, -and use the `setup-toolchain.sh` script to configure the appropriate toolchain -for the Clippy directory. +See the [Basics](basics.md#get-the-code) documentation. ## Getting Started @@ -113,7 +110,7 @@ For cargo lints, the process of testing differs in that we are interested in the `Cargo.toml` manifest file. We also need a minimal crate associated with that manifest. -If our new lint is named e.g. `foo_categories`, after running `cargo dev new_lint` +If our new lint is named e.g. `foo_categories`, after running `cargo dev new_lint` we will find by default two new crates, each with its manifest file: * `tests/ui-cargo/foo_categories/fail/Cargo.toml`: this file should cause the new lint to raise an error. diff --git a/doc/basics.md b/doc/basics.md new file mode 100644 index 00000000000..c1fd2fbcd1b --- /dev/null +++ b/doc/basics.md @@ -0,0 +1,106 @@ +# Basics for hacking on Clippy + +This document explains the basics for hacking on Clippy. Besides others, this +includes how to set-up the development environment, how to build and how to test +Clippy. For a more in depth description on the codebase take a look at [Adding +Lints] or [Common Tools]. + +[Adding Lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md +[Common Tools]: https://github.com/rust-lang/rust-clippy/blob/master/doc/common_tools_writing_lints.md + +- [Basics for hacking on Clippy](#basics-for-hacking-on-clippy) + - [Get the code](#get-the-code) + - [Setup](#setup) + - [Building and Testing](#building-and-testing) + - [`cargo dev`](#cargo-dev) + +## Get the Code + +First, make sure you have checked out the latest version of Clippy. If this is +your first time working on Clippy, create a fork of the repository and clone it +afterwards with the following command: + +```bash +git clone git@github.com:<your-username>/rust-clippy +``` + +If you've already cloned Clippy in the past, update it to the latest version: + +```bash +# upstream has to be the remote of the rust-lang/rust-clippy repo +git fetch upstream +# make sure that you are on the master branch +git checkout master +# rebase your master branch on the upstream master +git rebase upstream/master +# push to the master branch of your fork +git push +``` + +## Setup + +Next we need to setup the toolchain to compile Clippy. Since Clippy heavily +relies on compiler internals it is build with the latest rustc master. To get +this toolchain, you can just use the `setup-toolchain.sh` script or use +`rustup-toolchain-install-master`: + +```bash +sh setup-toolchain.sh +# OR +cargo install rustup-toolchain-install-master +rustup-toolchain-install-master -f -n master -c rustc-dev -c llvm-tools +rustup override set master +``` + +## Building and Testing + +Once the `master` toolchain is installed, you can build and test Clippy like +every other Rust project: + +```bash +cargo build # builds Clippy +cargo test # tests Clippy +``` + +Since Clippys test suite is pretty big, there are some commands that only run a +subset of Clippys tests: + +```bash +# only run UI tests +cargo uitest +# only run UI tests starting with `test_` +TESTNAME="test_" cargo uitest +# only run dogfood tests +cargo test --test dogfood +``` + +If the output of a UI test differs from the expected output, you can update the +reference file with: + +```bash +sh tests/ui/update-all-references.sh +``` + +For example, this is necessary, if you fix a typo in an error message of a lint +or if you modify a test file to add a test case. + +_Note:_ This command may update more files than you intended. In that case only +commit the files you wanted to update. + +## `cargo dev` + +Clippy has some dev tools to make working on Clippy more convenient. These tools +can be accessed through the `cargo dev` command. Available tools are listed +below. To get more information about these commands, just call them with +`--help`. + +```bash +# formats the whole Clippy codebase and all tests +cargo dev fmt +# register or update lint names/groups/... +cargo dev update_lints +# create a new lint and register it +cargo dev new_lint +# (experimental) Setup Clippy to work with rust-analyzer +cargo dev ra-setup +``` From 17903f6d7107c6d31ee15f4c46af29d1f4aa363f Mon Sep 17 00:00:00 2001 From: flip1995 <hello@philkrones.com> Date: Fri, 24 Jul 2020 17:48:43 +0200 Subject: [PATCH 2/3] Mention lint naming guidelines earlier --- doc/adding_lints.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/doc/adding_lints.md b/doc/adding_lints.md index d5f4f5d5659..168092f7329 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -35,12 +35,14 @@ There is a bit of boilerplate code that needs to be set up when creating a new lint. Fortunately, you can use the clippy dev tools to handle this for you. We are naming our new lint `foo_functions` (lints are generally written in snake case), and we don't need type information so it will have an early pass type -(more on this later on). To get started on this lint you can run -`cargo dev new_lint --name=foo_functions --pass=early --category=pedantic` -(category will default to nursery if not provided). This command will create -two files: `tests/ui/foo_functions.rs` and `clippy_lints/src/foo_functions.rs`, -as well as run `cargo dev update_lints` to register the new lint. For cargo lints, -two project hierarchies (fail/pass) will be created by default under `tests/ui-cargo`. +(more on this later on). If you're not sure if the name you chose fits the lint, +take a look at our [lint naming guidelines][lint_naming]. To get started on this +lint you can run `cargo dev new_lint --name=foo_functions --pass=early +--category=pedantic` (category will default to nursery if not provided). This +command will create two files: `tests/ui/foo_functions.rs` and +`clippy_lints/src/foo_functions.rs`, as well as run `cargo dev update_lints` to +register the new lint. For cargo lints, two project hierarchies (fail/pass) will +be created by default under `tests/ui-cargo`. Next, we'll open up these files and add our lint! From 3a4cc9f7f085e73fbfe57e8c896b90a5fe61c4f4 Mon Sep 17 00:00:00 2001 From: flip1995 <hello@philkrones.com> Date: Fri, 24 Jul 2020 23:17:52 +0200 Subject: [PATCH 3/3] Address review comments --- doc/basics.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/basics.md b/doc/basics.md index c1fd2fbcd1b..5c07d9b98a5 100644 --- a/doc/basics.md +++ b/doc/basics.md @@ -48,6 +48,7 @@ this toolchain, you can just use the `setup-toolchain.sh` script or use sh setup-toolchain.sh # OR cargo install rustup-toolchain-install-master +# For better IDE integration also add `-c rustfmt -c rust-src` (optional) rustup-toolchain-install-master -f -n master -c rustc-dev -c llvm-tools rustup override set master ``` @@ -62,8 +63,8 @@ cargo build # builds Clippy cargo test # tests Clippy ``` -Since Clippys test suite is pretty big, there are some commands that only run a -subset of Clippys tests: +Since Clippy's test suite is pretty big, there are some commands that only run a +subset of Clippy's tests: ```bash # only run UI tests @@ -74,7 +75,7 @@ TESTNAME="test_" cargo uitest cargo test --test dogfood ``` -If the output of a UI test differs from the expected output, you can update the +If the output of a [UI test] differs from the expected output, you can update the reference file with: ```bash @@ -87,6 +88,8 @@ or if you modify a test file to add a test case. _Note:_ This command may update more files than you intended. In that case only commit the files you wanted to update. +[UI test]: https://rustc-dev-guide.rust-lang.org/tests/adding.html#guide-to-the-ui-tests + ## `cargo dev` Clippy has some dev tools to make working on Clippy more convenient. These tools