After performance concers on the `nonminimal_bool` lint, we [performed a
crater run](https://github.com/rust-lang/rust/issues/153883)
This crater run revealed that in 19120 suggestions taken from 1.3
million crates, only about 36% had resulted in multi-terminal
suggestions (suggestions that were not only a single boolean).
This suggests that most cases of this lint firing, the expression that
triggered was pretty simple. And thus, we should not take such a huge
toll for a lint that isn't that useful.
In the future, a rewrite of that `bool_expr` function could be very
useful to alleviate the performance toll. Because clearly running the
Quine McCluskey algorithm on all those boolean operations is not ideal.
---
## Performance report:
`cargo clippy` -> 5,077,447,157 icount
`cargo check` -> 3,629,576,891 icount
So the Clippy that `rustup` delivers is about 1.4b instructions more
than `cargo check`.
`NonminimalBool` lint pass -> 318,169,034
$$\frac{nonminimal\\_bool}{clippy-check}*100 = 21.97496844$$
So, the `NonminimalBool` lint pass is about 22% of all Clippy-exclusive
instruction count.
---
After this change, we can now compare the Clippy BUILT FROM MASTER, NOT
DELIVERED BY RUSTUP, with the Clippy from this PR.
`master/target/release/cargo-clippy` -> 5,296,838,955 (Slower than
Rustup)
`pr/target/release/cargo-clippy` -> 4,977,035,941.
So, $master - PR = 319803014$, which is the exact icount of
`Nonminimal_bool` and about 22% of Clippy's whole execution time.
Profiled on `syn-2.0.71`, with Callgrind.
changelog:[`nonminimal_bool`]: Move to `pedantic`.
changelog:[`overly_complex_bool_expr`]: Move to `pedantic`.
This is going to be used as part of more thoroughly checking whether we
can change the type of an expression. e.g. `range_plus_one` suggests
changing `Range` to `RangeInclusive`.
changelog: none
In some cases, removing a cast that looks unnecessary can
still change type inference, even when the cast is to the
expected same type.
The lint now handles casts that flow through
intermediate expressions (e.g. calls, blocks, let
bindings, loops...). It also treats placeholder
generics (e.g. `::<_>`) as inference sensitive.
Added regression tests with similar behavior
as the original bug, some edge cases and
tests to check if false negative cases
weren't created.
Closesrust-lang/rust-clippy#16449
changelog: [`unnecessary_cast`]: fixed a suggestion to remove a cast
that can cause a compilation error if followed.
closes: rust-lang/rust-clippy#16768
I agree with the:
> importing it just to get the side effects of the trait being in scope
(anonymously, via as _) is not renaming it...
even without changing the name, we still not able to see it's original
name when calling a trait method, so I think this should be fine.
---
changelog: [`unsafe_removed_from_name`]: skip linting when renaming to
'_'
Closesrust-lang/rust-clippy#16751
changelog: [`question_mark`] fix FN for manual unwrap with `if let` or
`match`
changelog: [`question_mark`] fix FN when the match scrutinee is behind
reference
changelog: [`question_mark`] fix FN when match binding is behind `ref`
or `mut`
rustc doesn't keep around proc macro helper attributes in the HIR. So it is no
longer possible to lint this. In a LateLintPass the necessary information isn't
around anymore, in an EarlyPassLint derive attributes are already expanded and
dropped and it is not possible to relate an impl with the type it is implemented
on. Doing that with `ast::Path`s is brittle.
In some cases, removing a cast that looks unnecessary
can still change type inference, even when the cast is
to the expected same type.
The lint now handles casts that flow through
intermediate expressions (e.g. calls, blocks, let
bindings, loops...). It also treats placeholder
generics (e.g. `::<_>`) as inference sensitive.
Added regression tests with similar behavior
as the original bug, some edge cases and
tests to check if false negative cases
weren't created.
Closes#16449
*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16600)*
changelog: [`manual_option_zip`]: new lint that detects `a.and_then(|a|
b.map(|b| (a, b)))` and suggests using `a.zip(b)` instead
This PR introduces a new lint `manual_option_zip` that identifies cases
where `Option::and_then` is followed by `Option::map` in the provided
closure to construct a tuple that could be more concisely shaped using
`Option::zip`.
The lint detects the pattern:
```rust
a.and_then(|x| b.map(|y| (x, y)))
```
And suggests replacing it with:
```rust
a.zip(b)
```
Closesrust-lang/rust-clippy#16599
*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16687)*
Fixesrust-lang/rust-clippy#16639
### Description
This PR introduces a new lint manual\_noop\_waker that detects manual,
empty implementations of the std::task::Wake trait. These can be
replaced with std::task::Waker::noop(), which is more performant (avoids
Arc allocation) and cleaner.
### Example
```rust
struct MyWaker;
impl Wake for MyWaker {
fn wake(self: Arc<Self>) {}
fn wake_by_ref(self: &Arc<Self>) {}
}
````
Can be replaced with:
```rust
let waker = Waker::noop();
````
* \[x\] I've followed the [contribution
guidelines](https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md)
* \[x\] I've added UI tests for the new lint
* \[x\] I've run `cargo dev update_lints`
changelog: `[manual_noop_waker]`: Added a lint to detect manual no-op
Wake implementations.
*Please write a short comment explaining your change (or "none" for
internal only changes)*
Fixesrust-lang/rust-clippy#16110
changelog: [`similar_names`] : Changed the lint docs to reflect its
actual behavior
When a loop variable is unused (`for _ in 0..n`) and the counter is
being
incremented, Clippy previously suggested the generic zip-based rewrite.
This PR adds a more idiomatic `.take(n)` suggestion for the common case
where the range is half-open and starts at zero:
```rust
// before
for _ in 0..MAX {
base += 1;
}
// suggested
for base in (100..).take(MAX)
```
Non-zero starts (`5..MAX`) and inclusive ranges (`0..=MAX`) are
explicitly excluded and fall through to the existing zip suggestion.
Both cases are covered by new UI tests.
changelog: [`explicit_counter_loop`]: suggest `(init..).take(n)` when
loop variable is unused and range is `0..n`
fixesrust-lang/rust-clippy#16642
After performance concers on the `nonminimal_bool` lint, we performed a
crater run (rust-lang/rust issue number 153883).
This crater run revealed that in 19120 suggestions taken from 1.3
million crates, only about 36% had resulted in multi-terminal
suggestions (suggestions that were not only a single boolean).
This suggests that most cases of this lint firing, the expression that
triggered was pretty simple. And thus, we should not take such a huge
toll for a lint that isn't that useful.
changelog:[`nonminimal_bool`]: Move to `pedantic`.
changelog:[`overly_complex_bool_expr`]: Move to `pedantic`.
Add macro matcher for `guard` fragment specifier
Tracking issue #153104
This PR implements a new `guard` macro matcher to match `if-let` guards (specifically [`MatchArmGuard`](https://github.com/rust-lang/reference/blob/50a1075e879be75aeec436252c84eef0fad489f4/src/expressions/match-expr.md#match-guards)). In the upcoming PR, we can use this new matcher in the `matches!` and `assert_matches!` macros to support their use with `if-let` guards. (see #152313)
The original `Expr` used to represent a guard has been wrapped in a new `Guard` type, allowing us to carry the span information of the leading `if` keyword. However, it might be even better to include the `if` keyword in the `Guard` type as well? I've left a FIXME comment in the code.
fixes: rust-lang/rust-clippy#16742
properly select post operation for identity map according to map method
kind
changelog: [`iter_kv_map`]: handle identity map for `map` and `flat_map`
*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16683)*
Extend the lint to detect the case where the popped value is used, but
in such cases just emit the lint with no suggestion.
changelog: [`manual_pop_if`]: in case the popped value is used, just
emit the lint with no suggestion.
changelog: [`manual_pop_if`]: also cover `.pop().unwrap_unchecked()`
Ada Alakbarova