Fixes#4226
This introduces the lint await_holding_lock. For async functions, we iterate
over all types in generator_interior_types and look for types named MutexGuard,
RwLockReadGuard, or RwLockWriteGuard. If we find one then we emit a lint.
If let else mutex
changelog: Adds lint to catch incorrect use of `Mutex::lock` in `if let` expressions with lock calls in any of the blocks.
closes: #5219
add --fix support to `cargo-clippy`
Prior to this we had started work on integrating clippy as a subcommand directly into cargo in the form of `cargo clippy-preview` and `cargo fix --clippy`. In the course of that work it was decided that the best approach would be to strictly add the features clippy needed to cargo in order to insert `clippy-driver` only for workspace crates. This was accomplished by adding a `RUSTC_WORKSPACE_WRAPPER` env variable to cargo that will override the normal `RUSTC_WRAPPER` when both are present and the current crate is a workspace crate.
This change adds support to clippy to use this by setting the `RUSTC_WORKSPACE_WRAPPER` env variable instead `RUSTC_WRAPPER` and by detecting `--fix` as an arg and swapping out the `check` cargo command for `fix` when it is present.
WIP, here are the current issues that I still need to resolve
- [x] Detect if we're running on nightly rust
- [x] Set `RUSTC_WORKSPACE_WRAPPER` on nightly, and `RUSTC_WRAPPER` on stable
- [x] Error out on stable when `--fix` is specified, because stable currently hasn't landed the PR for `RUSTC_WORKSPACE_WRAPPER` so if we set this it just runs check and silently fails
- [ ] Update the help text
- [ ] The current plan is to shell out to `cargo check --help` and then postprocess the output to mention clippy instead of check where appropriate and to add the extra info about `--fix` and the `-- -A lint` options.
- [x] tests?
changelog: add `--fix` arg to `cargo-clippy`
Disallow bit-shifting in integer_arithmetic
Make the `integer_arithmetic` lint detect all the operations that are defined as being capable of overflow in the [Rust Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), by also linting for bit-shifting operations (`<<`, `>>`).
changelog: Disallow bit-shifting in `integer_arithmetic`
Add lint on large non scalar const
This PR adds the new lint `non_scalar_const` that aims to warn against `const` declaration of large arrays. For performance, because of inlining, large arrays should be preferably declared as `static`.
Note: i made this one to warn on all const arrays, whether they are in a body function or not. I don't know if this is really necessary, i could just reduce this lint to variables out of function scope.
Fixes: #400
changelog: add new lint for large non-scalar types declared as const
Downgrade implicit_hasher to pedantic
From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher), this lint is intended to suggest:
```diff
- pub fn foo(map: &mut HashMap<i32, i32>) { }
+ pub fn foo<S: BuildHasher>(map: &mut HashMap<i32, i32, S>) { }
```
I think this is pedantic. I get that this lint can benefit core libraries like serde, but that's exactly the use case for pedantic lints; a library like serde will [enable clippy_pedantic](fd6741f4b0/src/lib.rs (L304)) and take the time to go through everything possible. Similar for libraries doing a libz blitz style checkup before committing to a 1.0 release; it would make sense to run through all the available pedantic lints then.
But otherwise, for most codebases and certainly for industrial codebases, the above suggested change just makes the codebase more obtuse for questionable benefit.
changelog: Remove implicit_hasher from default set of enabled lints
Downgrade unreadable_literal to pedantic
As motivated by #5418. This is the top most commonly suppressed Clippy style lint, which indicates that the community has decided they don't share Clippy's opinion on the best style of this.
I've left the lint in as pedantic, though it could be that "restriction" would be better -- I can see this lint being useful as an opt-in restriction in some codebases.
changelog: Remove unreadable_literal from default set of enabled lints
Add new lint for `Result<T, E>.map_or(None, Some(T))`
Fixes#5414
PR Checklist
---
- [x] Followed lint naming conventions (the name is a bit awkward, but it seems to conform)
- [x] Added passing UI tests (including committed .stderr file)
- [x] cargo test passes locally
- [x] Executed cargo dev update_lints
- [x] Added lint documentation
- [x] Run cargo dev fmt
`Result<T, E>` has an [`ok()`](https://doc.rust-lang.org/std/result/enum.Result.html#method.ok) method that adapts a `Result<T,E>` into an `Option<T>`.
It's possible to get around this adapter by writing `Result<T,E>.map_or(None, Some)`.
This lint is implemented as a new variant of the existing [`option_map_none` lint](https://github.com/rust-lang/rust-clippy/pull/2128)
Downgrade inefficient_to_string to pedantic
From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string):
> ```diff
> - ["foo", "bar"].iter().map(|s| s.to_string());
>
> + ["foo", "bar"].iter().map(|&s| s.to_string());
> ```
I feel like saving 10 nanoseconds from the formatting machinery isn't worth asking the programmer to insert extra `&` / `*` noise in the *vast* majority of cases. This is a pedantic lint.
changelog: Remove inefficient_to_string from default set of enabled lints
Downgrade trivially_copy_pass_by_ref to pedantic
The rationale for this lint is documented as:
> In many calling conventions instances of structs will be passed through registers if they fit into two or less general purpose registers.
I think the purported performance benefits of clippy's recommendation are overstated. This isn't worth asking people to sprinkle code with more `*``*``&``*``&` to chase the alleged performance.
This should be a pedantic lint that is disabled by default and opted in if some specific performance sensitive codebase determines that it is worthwhile.
As a reminder, a typical place that a reference to a primitive would come up is if the function is used as a filter. Triggering a performance-oriented lint on this type of code is the definition of pedantic.
```rust
fn filter(_n: &i32) -> bool {
true
}
fn main() {
let v = vec![1, 2, 3];
v.iter().copied().filter(filter).for_each(drop);
}
```
```console
warning: this argument (4 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
--> src/main.rs:1:15
|
1 | fn filter(_n: &i32) -> bool {
| ^^^^ help: consider passing by value instead: `i32`
```
changelog: Remove trivially_copy_pass_by_ref from default set of enabled lints
Downgrade let_unit_value to pedantic
Given that the false positive in #1502 is marked E-hard and I don't have much hope of it getting fixed, I think it would be wise to disable this lint by default. I have had to suppress this lint in every substantial codebase (\>100k line) I have worked in. Any time this lint is being triggered, it's always the false positive case.
The motivation for this lint is documented as:
> A unit value cannot usefully be used anywhere. So binding one is kind of pointless.
with this example:
> ```rust
> let x = {
> 1;
> };
> ```
Sure, but the author would find this out via an unused_variable warning or from `x` not being the type that they need further down. If there ends up being a type error on `x`, clippy's advice isn't going to help get the code compiling because it can only run if the code already compiles.
changelog: Remove let_unit_value from default set of enabled lints
Result<T, E> has an `ok()` method that adapts a Result<T,E> into an Option<T>.
It's possible to get around this adapter by writing Result<T,E>.map_or(None, Some).
This lint is implemented as a new variant of the existing
[`option_map_none` lint](https://github.com/rust-lang/rust-clippy/pull/2128)
