|
| 1 | +--- |
| 2 | +title: Getting started with Clippy on an existing project |
| 3 | +timestamp: 2024-01-17T17:30:01 |
| 4 | +published: true |
| 5 | +description: Clippy can provide a lot of valuable improvements for your code, but it is hard to add it to an existing project. |
| 6 | +tags: |
| 7 | + - Clippy |
| 8 | + - lint |
| 9 | +--- |
| 10 | + |
| 11 | + |
| 12 | +[Clippy](https://doc.rust-lang.org/stable/clippy/index.html) can provide a lot of valuable improvements for your code, but it is hard to add it to an existing project. |
| 13 | + |
| 14 | +In any Crate where you have the standard Rust toolchain you can run `cargo clippy` and get numerous complaints. |
| 15 | + |
| 16 | +Once you fix all the simple lints, you can go to the [documentation of Clippy](https://doc.rust-lang.org/stable/clippy/index.html) and find more powerful spells, err, lints. |
| 17 | +To make life easier there are also some lints grouped together. |
| 18 | + |
| 19 | +For example, I decided to be pedantic and enabled the `pedantic` lint group by adding the following to `Cargo.toml`: |
| 20 | + |
| 21 | +```toml |
| 22 | +[lints.clippy] |
| 23 | +pedantic = "deny" |
| 24 | +``` |
| 25 | + |
| 26 | +running `cargo clippy` after this gave me some 60 failures. Very annoying. A better way would have been to start the project with these lints already enabled, but oh well. |
| 27 | +That time is gone. |
| 28 | + |
| 29 | +So what I did is disabled the individual lints one-by-one and had the following in `Cargo.toml`: |
| 30 | + |
| 31 | + |
| 32 | +```toml |
| 33 | +[lints.clippy] |
| 34 | +pedantic = "deny" |
| 35 | +must_use_candidate = "allow" |
| 36 | +manual_string_new = "allow" |
| 37 | +needless_pass_by_value = "allow" |
| 38 | +no_effect_underscore_binding = "allow" |
| 39 | +missing_errors_doc = "allow" |
| 40 | +missing_panics_doc = "allow" |
| 41 | +module_name_repetitions = "allow" |
| 42 | +uninlined_format_args = "allow" |
| 43 | + |
| 44 | +#unwrap_used = "deny" |
| 45 | +#unwrap_or_default = "deny" |
| 46 | +``` |
| 47 | + |
| 48 | +This **almost** worked. This disabled all the lint errors **I** received in my code-base, except the ones that violated the `uninlined_format_args` lint. |
| 49 | +I was baffled and opened and [issue](https://github.com/rust-lang/rust-clippy/issues/12161). I got a quick response that the proper way to do this is to |
| 50 | +set the priority of the `pedantic` lint manually. So I ended up with this configuration: (see the change in the 2nd row). |
| 51 | + |
| 52 | + |
| 53 | +```toml |
| 54 | +[lints.clippy] |
| 55 | +pedantic = { priority = -1, level = "deny" } |
| 56 | +must_use_candidate = "allow" |
| 57 | +manual_string_new = "allow" |
| 58 | +needless_pass_by_value = "allow" |
| 59 | +no_effect_underscore_binding = "allow" |
| 60 | +missing_errors_doc = "allow" |
| 61 | +missing_panics_doc = "allow" |
| 62 | +module_name_repetitions = "allow" |
| 63 | +uninlined_format_args = "allow" |
| 64 | + |
| 65 | +#unwrap_used = "deny" |
| 66 | +#unwrap_or_default = "deny" |
| 67 | +``` |
| 68 | + |
| 69 | +As you can see I also have two lints at the end commented out. These and probably some other lints are not part of the `pedantic` group, but I think I will want to enable them later. |
| 70 | +Right now they have too many issues to report. |
| 71 | + |
| 72 | + |
| 73 | +## Fixing the lints one-by-one |
| 74 | + |
| 75 | +My approach at this point is to go over the individual lints that I've disabled. Pick one that looks the most important (or the easiest to fix), enable it by removing |
| 76 | +the allow-line from `Cargo.toml` and fix all the code where I have this issue. |
| 77 | + |
| 78 | +I personally find it easier to fix similar issues throughout the code. |
| 79 | + |
| 80 | +Of course I can do that relatively safely as I have extensive tests for the code, so I know that chance of introducing a bug without a test failing is rather slim. |
| 81 | + |
| 82 | + |
| 83 | +## Pre-commit |
| 84 | + |
| 85 | +In order to enforce the rules I even added running clippy to the [Pre-commit](https://pre-commit.com/) configuration file and to the GitHub Actions CI configuration. |
| 86 | + |
| 87 | +`.pre-commit-config.yaml` like this: |
| 88 | + |
| 89 | +``` |
| 90 | + - repo: local |
| 91 | + hooks: |
| 92 | + - id: cargo-clippy |
| 93 | + name: cargo clippy |
| 94 | + language: system |
| 95 | + entry: cargo clippy -- --deny warnings |
| 96 | + always_run: true |
| 97 | + pass_filenames: false |
| 98 | + files: \.rs$ |
| 99 | +``` |
| 100 | + |
| 101 | +## GitHub Actions CI |
| 102 | + |
| 103 | + |
| 104 | +``` |
| 105 | +jobs: |
| 106 | + build: |
| 107 | +
|
| 108 | + runs-on: ubuntu-latest |
| 109 | +
|
| 110 | + steps: |
| 111 | + - uses: actions/checkout@v3 |
| 112 | +
|
| 113 | + - name: Clippy stop at any warning |
| 114 | + run: cargo clippy -- --deny warnings |
| 115 | +``` |
| 116 | + |
| 117 | +## Conclusion |
| 118 | + |
| 119 | +Adding Clippy or extending the Clippy lints in an existing project is not easy and might not feel as productive work, but once can try to do that step-by-step during |
| 120 | +a longer period of time and that will help smooth the introduction of the lints and help improve the code-base. |
| 121 | + |
| 122 | + |
| 123 | + |
| 124 | + |
| 125 | + |
| 126 | + |
| 127 | + |
0 commit comments