[`len_without_is_empty`]: follow type alias to find inherent `is_empty` method
Fixes#11165
When we see an `impl B` and `B` is a type alias to some type `A`, then we need to follow the type alias to look for an `is_empty` method on the aliased type `A`. Before this PR, it'd get the inherent impls of `B`, which there aren't any and so it would warn that there isn't an `is_empty` method even if there was one.
Passing the type alias `DefId` to `TyCtxt::type_of` gives us the aliased `DefId` (or simply return the type itself if it wasn't a type alias) so we can just use that
changelog: [`len_without_is_empty`]: follow type alias to find inherent `is_empty` method
[`implied_bounds_in_impls`]: include (previously omitted) associated types in suggestion
Fixes#11435
It now includes associated types from the implied bound that were omitted in the second bound. Example:
```rs
fn f() -> impl Iterator<Item = u8> + ExactSizeIterator> {..}
```
Suggestion before this change:
```diff
- pub fn my_iter() -> impl Iterator<Item = u32> + ExactSizeIterator {
+ pub fn my_iter() -> impl ExactSizeIterator {
```
It didn't include `<Item = u32>` on `ExactSizeIterator`. Now, with this change, it does.
```diff
- pub fn my_iter() -> impl Iterator<Item = u32> + ExactSizeIterator {
+ pub fn my_iter() -> impl ExactSizeIterator<Item = u32> {
```
We also now extend the span to include not just possible `+` ahead of it, but also behind it (an example for this is in the linked issue as well).
**Note:** The overall diff is a bit noisy, because building up the suggestion involves quite a bit more logic now and I decided to extract that into its own function. For that reason, I split this PR up into two commits. The first commit contains the actual "logic" changes. Second commit just moves code around.
changelog: [`implied_bounds_in_impls`]: include (previously omitted) associated types in suggestion
changelog: [`implied_bounds_in_impls`]: include the `+` behind bound if it's the last bound
Rename incorrect_impls to non_canonical_impls, move them to warn by default
The wording/category of these feel too strong to me, I would expect most of the time it's linting the implementations aren't going to be *incorrect*, just unnecessary
changelog: rename `incorrect_clone_impl_on_copy_type` to [`non_canonical_clone_impl`]
changelog: rename `incorrect_partial_ord_impl_on_ord_type` to [`non_canonical_partial_ord_impl`]
changelog: Move [`non_canonical_clone_impl`], [`non_canonical_partial_ord_impl`] to suspicious
Preserve literals and range kinds in `manual_range_patterns`
Fixes#11461
Also enables linting when there are 3 or fewer alternatives if one of them is already a range pattern
changelog: none
[`slow_vector_initialization`]: use the source span of vec![] macro and fix another FP
Fixes#11408
<details>
<summary>Also fixes a FP when the vec initializer comes from a macro other than `vec![]`</summary>
```rs
macro_rules! x {
() => { vec![] }
}
fn f() {
let mut v = x!();
v.resize(10, 0);
}
```
This shouldn't warn. The `x!` macro might be doing other things, so just replacing `x!()` with `vec![0; 10]` is not always an option.
</details>
I added some test cases for macro expansions, however I don't think there's a way to write a test for that specific warning that appeared in the linked issue. As far as I understand, that happens when the rust-src rustup component isn't installed (so the stdlib source is unavailable) and the span points to the `vec![]` *expansion*, instead of the `vec![]` that the user wrote.
changelog: [`slow_vector_initialization`]: use the source span of `vec![]` macro
changelog: [`slow_vector_initialization`]: only warn on `vec![]` expansions and allow other macros
skip `todo!()` in `never_loop`
As promised in #11450, here is an implementation which skips occurrences of the `todo!()` macro.
changelog: [`never_loop`]: skip loops containing `todo!()`
Don't pass extra generic arguments in `needless_borrow`
fixes#10253
Also switches to using `implements_trait` which does ICE when clippy's debug assertions are enabled.
changelog: None
[`implied_bounds_in_impls`]: don't ICE on default generic parameter and move to nursery
Fixes#11422
This fixes two ICEs ([1](https://github.com/rust-lang/rust-clippy/issues/11422#issue-1872351763), [2](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2901e6febb479d3bd2a74f8a5b8a9305)), and moves it to nursery for now, because this lint needs some improvements in its suggestion (see #11435, for one such example).
changelog: Moved [`implied_bounds_in_impls`] to nursery (Now allow-by-default)
[#11437](https://github.com/rust-lang/rust-clippy/pull/11437)
changelog: [`implied_bounds_in_impls`]: don't ICE on default generic parameter in supertrait clause
r? `@xFrednet` (since you reviewed my PR that added this lint, I figured it might make sense to have you review this as well since you have seen this code before. If you don't want to review this, sorry! Feel free to reroll then)
--------
As for the ICE, it's pretty complicated and very confusing imo, so I'm going to try to explain the idea here (partly for myself, too, because I've confused myself several times writing- and fixing this):
<details>
<summary>Expand</summary>
The general idea behind the lint is that, if we have this function:
```rs
fn f() -> impl PartialEq<i32> + PartialOrd<i32> { 0 }
```
We want to lint the `PartialEq` bound because it's unnecessary. That exact bound is already specified in `PartialOrd<i32>`'s supertrait clause:
```rs
trait PartialOrd<Rhs>: PartialEq<Rhs> {}
// PartialOrd<i32>: PartialEq<i32>
```
The way it does this is in two steps:
- Go through all of the bounds in the `impl Trait` return type and collect each of the trait's supertrait bounds into a vec. We also store the generic arguments for later.
- `PartialEq` has no supertraits, nothing to add.
- `PartialOrd` is defined as `trait PartialOrd: PartialEq`, so add `PartialEq` to the list, as well as the generic argument(s) `<i32>`
Once we are done, we have these entries in the vec: `[(PartialEq, [i32])]`
- Go through all the bounds again, and looking for those bounds that have their trait `DefId` in the implied bounds vec.
- `PartialEq` is in that vec. However, that is not enough, because the trait is generic. If the user wrote `impl PartialEq<String> + PartialOrd<i32>`, then `PartialOrd` clearly doesn't imply `PartialEq`. Which means, we also need to check that the generic parameters match. This is why we also collected the generic arguments in `PartialOrd<i32>`. This process of checking generic arguments is pretty complicated and is also where the two ICEs happened.
The way it checks that the generic arguments match is by comparing the generic parameters in the super trait clause:
```rs
trait PartialOrd<Rhs>: PartialEq<Rhs> {}
// ^^^^^^^^^^^^^^
```
...this needs to match...
```rs
fn f() -> impl PartialEq<i32> + ...
// ^^^^^^^^^^^^^^
```
In the compiler, the `Rhs` generic parameter is its own type and we cannot just compare it to `i32`. We need to "substitute" it.
Internally, `Rhs` is represented as `Rhs#1` (the number next to # represents the type parameter index. They start at 0, but 0 is "reserved" for the implicit `Self` generic parameter).
How do we go from `Rhs#1` to `i32`? Well, we know that all the generic parameters had to be substituted in the `impl ... + PartialOrd<i32>` type. So we subtract 1 from the type parameter index, giving us 0 (`Self` is not specified in that list of arguments). We use that as the index into the generic argument list `<i32>`. That's `i32`. Now we know that the supertrait clause looks like `: PartialEq<i32>`.
Then, we can compare that to what the user actually wrote on the bound that we think is being implied: `impl PartialEq<i32> + ...`.
Now to the actual bug: this whole logic doesn't take into account *default* generic parameters. Actually, `PartialOrd` is defined like this:
```rs
trait PartialOrd<Rhs = Self>: PartialEq<Rhs> {}
```
If we now have a function like this:
```rs
fn f() -> impl PartialOrd + PartialEq {}
```
that logic breaks apart... We look at the supertrait predicate `: PartialEq<Rhs>` (`Rhs` is `Rhs#1`), then take the first argument in the generic argument list `PartialEq<..>` to resolve the `Rhs`, but at this point we crash because there *is no* generic argument.
The index 0 is out of bounds. If this happens (and we even get to linting here, which could only happen if it passes typeck), it must mean that that generic parameter has a default type that is not required to be specified.
This PR changes the logic such that if we have a type parameter index that is out of bounds, it looks at the definition of the trait and check that there exists a default type that we can use instead.
So, we see `<Rhs = Self>`, and use `Self` for substitution, and end up with this predicate: `: PartialEq<Self>`. No crash this time.
</details>
Also stabilizes saturating_int_assign_impl, gh-92354.
And also make pub fns const where the underlying saturating_*
fns became const in the meantime since the Saturating type was
created.
Fix span when linting `explicit_auto_deref` immediately after `needless_borrow`
fixes#11366
changelog: `explicit_auto_deref`: Fix span when linting immediately after `needless_borrow`
Add config flag for reborrows in explicit_iter_loop
This PR adds a config flag for enforcing explicit into iter lint for reborrowed values. The config flag, `enforce_iter_loop_reborrow`, can be added to clippy.toml files to enable the linting behaviour. By default the reborrow lint is disabled.
fixes: #11074
changelog: [`explicit_iter_loop`]: add config flag `enforce_iter_loop_reborrow` to disable reborrow linting by default
new lint: `iter_out_of_bounds`
Closes#11345
The original idea in the linked issue seemed to be just about arrays afaict, but I extended this to catch some other iterator sources such as `iter::once` or `iter::empty`.
I'm not entirely sure if this name makes a lot of sense now that it's not just about arrays anymore (specifically, not sure if you can call `.take(1)` on an `iter::Empty` to be "out of bounds"?).
changelog: [`iter_out_of_bounds`]: new lint
[`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`
Closes#11371
This turned out to be a little more code than I originally thought, because the lint also makes sure to not lint if the user tries to mutate the option:
```rs
if option.is_some() {
option = None;
option.unwrap(); // don't lint here
}
```
... which means that even if we taught this lint to recognize `.as_mut()`, it would *still* not lint because that would count as a mutation. So we need to allow `.as_mut()` calls but reject other kinds of mutations.
Unfortunately it doesn't look like this is possible with `is_potentially_mutated` (seeing what kind of mutation happened).
This replaces it with a custom little visitor that does basically what it did before, but also allows `.as_mut()`.
changelog: [`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`
skip float_cmp check if lhs is a custom type
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`float_cmp`]: allow float eq comparison when lhs is a custom type that implements PartialEq<f32/f64>
If the lhs of a comparison is not float, it means there is a user implemented PartialEq, and the caller is invoking that custom version of `==`, instead of the default floating point equal comparison.
People may wrap f32 with a struct (say `MyF32`) and implement its PartialEq that will do the `is_close()` check, so that `MyF32` can be compared with either f32 or `MyF32`.
[`if_then_some_else_none`]: look into local initializers for early returns
Fixes#11394
As the PR title says, problem was that it only looked for early returns in semi statements. Local variables don't count as such, so it didn't count `let _v = x?;` (or even just `let _ = return;`) as a possible early return and didn't realize that it can't lint then.
Imo the `stmts_contains_early_return` function that was used before is redundant. `contains_return` could already do that if we just made the parameter a bit more generic, just like `for_each_expr`, which can already accept `&[Stmt]`
changelog: [`if_then_some_else_none`]: look into local initializers for early returns
This commit adds a config flag for enforcing explicit into iter lint
for reborrowed values. The config flag, enforce_iter_loop_reborrow, can be
added to clippy.toml files to enable the linting behaviour. By default
the lint is not enabled.
fix the uitest `enum_clike_unportable_variant`
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: none
fix "derivable_impls: attributes are ignored"
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`derivable_impls`]: allow the lint when the trait-impl methods has any attribute.
Added new lint: `reserve_after_initialization`
Closes https://github.com/rust-lang/rust-clippy/issues/11330.
A new lint that informs the user about a more concise way to create a vector with a known capacity.
Example:
```rust
let mut v: Vec<usize> = vec![];
v.reserve(10);
```
Produces the following help:
```rust
|
2 | / let mut v: Vec<usize> = vec![];
3 | | v.reserve(10);
| |__________________^ help: consider using `Vec::with_capacity(space_hint)`: `let v: Vec<usize> = Vec::with_capacity(10);`
|
```
And can be rewritten as:
```rust
let v: Vec<usize> = Vec::with_capacity(10);
```
changelog: new lint [`reserve_after_initialization`]
Fix tuple_array_conversions lint on nightly
```
changelog: ICE: [`tuple_array_conversions`]: Don't expect array length to always be usize
```
tl;dr: changed [`Const::eval_target_usize`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_middle/src/ty/consts.rs#L359) to [`Consts::try_eval_target_usize`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_middle/src/ty/consts.rs#L327) to get rid of ICE.
I have encountered a problem with clippy: it caught ICE when working with a codebase that uses a lot of nightly features.
Here's a (stripped) ICE info:
```
error: internal compiler error: /rustc/5c6a7e71cd66705c31c9af94077901a220f0870c/compiler/rustc_middle/src/ty/consts.rs:361:32: expected usize, got Const { ty: usize, kind: N/#1 }
thread 'rustc' panicked at /rustc/5c6a7e71cd66705c31c9af94077901a220f0870c/compiler/rustc_errors/src/lib.rs:1635:9:
Box<dyn Any>
stack backtrace:
...
16: 0x110b9c590 - rustc_middle[449edf845976488d]::util:🐛:bug_fmt
17: 0x102f76ae0 - clippy_lints[71754038dd04c2d2]::tuple_array_conversions::all_bindings_are_for_conv
...
```
I don't really know what's going on low-level-wise, but seems like this lin assumed that the length of the array can always be treated as `usize`, and *I assume* this doesn't play well with `feat(generic_const_exprs)`.
I wasn't able to build a minimal reproducible example, but locally this fix does resolve the issue.
key idea:
for `f` in `.map(f)` and `.for_each(f)`:
1. `f` must be a closure with one parameter
2. don't lint if mutable paramter in clsure `f`: `|mut x| ...`
3. don't lint if parameter is moved
[new_without_default]: include `where` clause in suggestions, make applicable
changelog: [`new_without_default`]: include `where` clause in suggestions
Correctly handle async blocks for NEEDLESS_PASS_BY_REF_MUT
Fixes https://github.com/rust-lang/rust-clippy/issues/11299.
The problem was that the `async block`s are popping a closure which we didn't go into, making it miss the mutable access to the variables.
cc `@Centri3`
changelog: none
[`useless_conversion`]: only lint on paths to fn items and fix FP in macro
Fixes#11065 (which is actually two issues: an ICE and a false positive)
It now makes sure that the function call path points to a function-like item (and not e.g. a `const` like in the linked issue), so that calling `TyCtxt::fn_sig` later in the lint does not ICE (fixes https://github.com/rust-lang/rust-clippy/issues/11065#issuecomment-1616836099).
It *also* makes sure that the expression is not part of a macro call (fixes https://github.com/rust-lang/rust-clippy/issues/11065#issuecomment-1616919639). ~~I'm not sure if there's a better way to check this other than to walk the parent expr chain and see if any of them are expansions.~~ (edit: it doesn't do this anymore)
changelog: [`useless_conversion`]: fix ICE when call receiver is a non-fn item
changelog: [`useless_conversion`]: don't lint if argument is a macro argument (fixes a FP)
r? `@llogiq` (reviewed #10814, which introduced these issues)
Correctly handle async blocks for NEEDLESS_PASS_BY_REF_MUT
Fixes https://github.com/rust-lang/rust-clippy/issues/11299.
The problem was that the `async block`s are popping a closure which we didn't go into, making it miss the mutable access to the variables.
cc `@Centri3`
changelog: none
[`useless_conversion`]: only lint on paths to fn items and fix FP in macro
Fixes#11065 (which is actually two issues: an ICE and a false positive)
It now makes sure that the function call path points to a function-like item (and not e.g. a `const` like in the linked issue), so that calling `TyCtxt::fn_sig` later in the lint does not ICE (fixes https://github.com/rust-lang/rust-clippy/issues/11065#issuecomment-1616836099).
It *also* makes sure that the expression is not part of a macro call (fixes https://github.com/rust-lang/rust-clippy/issues/11065#issuecomment-1616919639). ~~I'm not sure if there's a better way to check this other than to walk the parent expr chain and see if any of them are expansions.~~ (edit: it doesn't do this anymore)
changelog: [`useless_conversion`]: fix ICE when call receiver is a non-fn item
changelog: [`useless_conversion`]: don't lint if argument is a macro argument (fixes a FP)
r? `@llogiq` (reviewed #10814, which introduced these issues)
Use ui_test's Windows path backslash heuristic
changelog: none
Instead of unconditionally replacing `\` with `/` we now use [`Match::PathBackslash`](https://docs.rs/ui_test/latest/ui_test/enum.Match.html#variant.PathBackslash) to only replace backslashes in paths that look like windows paths
`ui-toml` and `ui-cargo` tests still use the old way because they produce verbatim paths on windows in some tests (`\\?\C:\foo\...`) which was finnicky to get the replacement order correct with
Also removes the `ui_test` -> `compiletest` alias and `VarGuard`
redundant_locals: fix FPs on mutated shadows
Fixes#11290.
When a mutable binding is shadowed by
a mutable binding of the same name in a different scope, mutations in that scope have different meaning.
This PR fixes spurious `redundant_locals` emissions on such locals.
cc `@Centri3,` `@flip1995`
changelog: [`redundant_locals`]: fix false positives on mutated shadows
Rustup
r? `@ghost`
cc `@max-niederman` With the latest sync, I'm getting a lot of FP in the `redundant_locals` lint you recently added. Any ideas where this could come from?
changelog: none
When a mutable binding is shadowed by
a mutable binding of the same name in a different scope,
mutations in that scope have different meaning.
This commit fixes spurious `redundant_locals` emissions
on such locals.
[`redundant_guards`]: don't lint on float literals
Fixes#11304
changelog: [`redundant_guards`]: don't lint on float literals
r? `@Centri3` i figured you are probably a good reviewer for this since you implemented the lint ^^
redundant_type_annotations: only pass certain def kinds to type_of
Fixes#11190Fixesrust-lang/rust#113516
Also adds an `is_lint_allowed` check to skip the lint when it's not needed
changelog: none
Fix `suspicious_xor_used_as_pow.rs` performance
The original `suspicious_xor_used_as_pow` lint had poor performance, so I fixed that + a little refactor so that module is readable.
**107 millis. -> 106 millis.** Using `SPEEDTEST` on Rust's VMs
fix#11060
changelog: [`suspicious_xor_used_as_pow`]: Improve performance by 0.934%
New lint `ignored_unit_patterns`
This idea comes from #11238. I've put the lint in `pedantic` as it might trigger numerous positives (three in Clippy itself).
changelog: [`ignored_unit_patterns`]: new lint
Suppress `question_mark` warning if `question_mark_used` is not allowed
Closes#11283
changelog: [`question_mark`]: Don't lint if `question_mark_used` is not allowed
Rename and allow `cast_ref_to_mut` lint
This PR is a small subset of https://github.com/rust-lang/rust/pull/112431, that is the renaming of the lint (`cast_ref_to_mut` -> `invalid_reference_casting`).
BUT also temporarily change the default level of the lint from deny-by-default to allow-by-default until https://github.com/rust-lang/rust/pull/112431 is merged.
r? `@Nilstrieb`
new lint: [`readonly_write_lock`]
Closes#8555
A new lint that catches `RwLock::write` calls to acquire a write lock only to read from it and not actually do any writes (mutations).
changelog: new lint: [`readonly_write_lock`]
Now `option_env_unwrap` warns even if a variable isn't set at compiletime
Fixes#10742
changelog: Fix false negative where `option_env_unwrap` wouldn't warn if the env variable isn't set at compile-time.
[`slow_vector_initialization`]: catch `Vec::new()` followed by `.resize(len, 0)`
Closes#10938
changelog: [`slow_vector_initialization`]: catch `Vec::new()` followed by `.resize(len, 0)`
New lint [`needless_return_with_try`]
Closes#10902
Rather than having a config option, this will just suggest removing the "return"; if `try_err` is used as well, then it'll be added again but without the `?`.
changelog: New lint [`needless_return_with_try`]
ptr_arg should ignore extern functions
Fixes: #11181
changelog: [`ptr_arg`]: ignore extern functions that are not
I am not sure whether we should ignore other Rust calling conventions like `rust-intrinsic`, `rust-call` or `rust-cold`.
`unwrap_or_else_default` -> `unwrap_or_default` and improve resulting lint
Resolves#10080 (though it doesn't implement exactly what's described there)
This PR does the following:
1. Merges `unwrap_or_else_default.rs`'s code into `or_fun_call.rs`
2. Extracts the code to handle `unwrap_or(/* default value */)` and similar, and moves it into `unwrap_or_else_default`
3. Implements the missing functionality from #9342, e.g.,, to handle `or_insert_with(Default::default)`
4. Renames `unwrap_or_else_default` to `unwrap_or_default` (since the "new" lint handles both `unwrap_or` and `unwrap_or_else`, it seemed sensible to use the shortened name)
This PR is currently two commits. The first implements 1-3, the second implements 4.
A word about 2: the `or_fun_call` lint currently produces warnings like the following:
```
error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:56:14
|
LL | with_new.unwrap_or(Vec::new());
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
```
To me, such warnings look like they should come from `unwrap_or_else_default`, not `or_fun_call`, especially since `or_fun_call` is [in the nursery](https://github.com/rust-lang/rust-clippy/pull/9829).
---
changelog: Move: Renamed `unwrap_or_else_default` to [`unwrap_or_default`]
[#10120](https://github.com/rust-lang/rust-clippy/pull/10120)
changelog: Enhancement: [`unwrap_or_default`]: Now handles more functions, like `or_insert_with`
[#10120](https://github.com/rust-lang/rust-clippy/pull/10120)
<!-- changelog_checked-->
check that the types are equal in `SpanlessEq::eq_expr`
Fixes#11213
changelog: [`if_same_then_else`]: don't lint for integer literals of different types
Fix async functions handling for `needless_pass_by_ref_mut` lint
Fixes https://github.com/rust-lang/rust-clippy/issues/11179.
The problem with async is that "internals" are actually inside a closure from the `ExprUseVisitor` point of view, meaning we need to actually run the check on the closures' body as well.
changelog: none
r? `@llogiq`
Make `comparison_to_empty` work on `if let`/`let` chains
This adds `LetChain` to `clippy_utils::higher`, other lints may benefit from such a change as well :D
changelog: Enhancement: [`comparison_to_empty`]: Now lints on `if let`
[`unused_async`]: don't lint if paths reference async fn without immediate call
Fixes#9695Fixes#9359
Clippy shouldn't lint unused `async` if there are paths referencing them if that path isn't the receiver of a function call, because that means that the function might be passed to some other function:
```rs
async fn f() {} // No await statements, so unused at this point
fn requires_fn_future<F: Future<Output = ()>>(_: fn() -> F) {}
requires_fn_future(f); // `f`'s asyncness is actually not unused.
```
(This isn't limited to just passing the function as a parameter to another function, it could also first be stored in a variable and later passed to another function as an argument)
This requires delaying the linting until post-crate and collecting path references to local async functions along the way.
changelog: [`unused_async`]: don't lint if paths reference async fn that require asyncness
fix dogfood lints in `redundant_local`
keep `redundant_local` from running in proc macros
rewrite `redundant_local` as late pass
make redundant_local's `find_binding` more readable
pluralize `redundant_locals` name
add test for `redundant_locals` in macros
test `redundant_locals` in proc macros
use more destructuring in `redundant_locals`
fix: format redundant_locals.rs
ignore needless_pass_by_mut_ref in redundant_locals test
Allow `Self::cmp(self, other)` as a correct impl
Fixes#11178
Also no longer checks if the method name is *just* cmp, but the path. That was an oversight on my part ^^
r? `@xFrednet`
(and `@blyxyas` too!)
changelog: [`incorrect_partial_ord_impl_on_ord_type`]: Now allows non-method calls to `cmp` like `Self::cmp(self, other)`
fix: false positive for `option_env!` in `ifs_same_cond`
Clippy had a false positive for with `ifs_same_cond` when two if-let expressions have an `option_env!` macro. The fix is similar to the `env!` macro fix.
The following example had a clippy error:
```rust
if let Some(env1) = option_env!("ENV1") {
// ...
} else if let Some(env2) = option_env!("ENV2") {
// ...
}
```
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01b85c61b56ddd900117fb247af04824
changelog: [`ifs_same_cond`]: fix false positive when using `option_env!` in if-let expressions.
[`unnecessary_literal_unwrap`]: Fix ICE on None.unwrap_or_default()
Fixes#11099Fixes#11064
I'm running into #11099 (cc `@y21)` on my Rust codebase. Clippy ICEs on this code when evaluating the `unnecessary_literal_unwrap` lint:
```rust
fn main() {
let val1: u8 = None.unwrap_or_default();
}
```
This fixes that ICE and adds an message specifically for that case:
```
error: used `unwrap_or_default()` on `None` value
--> $DIR/unnecessary_literal_unwrap.rs:26:5
|
LL | None::<String>.unwrap_or_default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap_or_default()`: `String::default()`
```
This PR also fixes the same ICE with `None.unwrap_or_else` (by giving the generic error message for the lint in that case).
changelog: Fix ICE in `unnecessary_literal_unwrap` on `None.unwrap_or_default()`
Clippy had a false positive for with `ifs_same_cond` when two
if-let expressions have an `option_env!` macro. The fix is similar to the
`env!` macro fix.
The following example had a clippy error:
```rust
if let Some(env1) = option_env!("ENV1") {
// ...
} else if let Some(env2) = option_env!("ENV2") {
// ...
}
```
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01b85c61b56ddd900117fb247af04824
changelog: Fix [`ifs_same_cond`] false positive when using `option_env!` in if-let expressions.
[`manual_filter_map`]: lint on `matches` and pattern matching
Fixes#8010
Previously this lint only worked specifically for a very limited set of methods on the filter call (`.filter(|opt| opt.is_some())` and `.filter(|res| res.is_ok())`). This PR extends it to also recognize `matches!` in the `filter` and pattern matching with `if let` or `match` in the `map`.
Example:
```rs
enum Enum {
A(i32),
B,
}
let _ = [Enum::A(123), Enum::B].into_iter()
.filter(|x| matches!(x, Enum::A(_)))
.map(|x| if let Enum::A(s) = x { s } else { unreachable!() });
```
Now suggests:
```diff
- .filter(|x| matches!(x, Enum::A(_))).map(if let Enum::A(s) = x { s } else { unreachable!() })
+ .filter_map(|x| match x { Enum::A(s) => Some(s), _ => None })
```
Adding this required a somewhat large change in code because it originally seemed to be specifically written with only method calls in the filter in mind, and `matches!` has different behavior in the map, so this new setup should make it possible to support more "generic" cases that need different handling for the filter and map calls.
changelog: [`manual_filter_map`]: lint on `matches` and pattern matching (and some internal refactoring)
Fix `unwrap_or_else_default` false positive
This PR fixes a false positive in the handling of `unwrap_or_else` with a default value when the value is needed for type inference.
An easy example to exhibit the false positive is the following:
```rust
let option = None;
option.unwrap_or_else(Vec::new).push(1);
```
The following code would not compile, because the fact that the value is a `Vec` has been lost:
```rust
let option = None;
option.unwrap_or_default().push(1);
```
The fix is to:
- implement a heuristic to tell whether an expression's type can be determined purely from its subexpressions, and the arguments and locals they use;
- apply the heuristic to `unwrap_or_else`'s receiver.
The heuristic returns false when applied to `option` in the above example, but it returns true when applied to `option` in either of the following examples:
```rust
let option: Option<Vec<u64>> = None;
option.unwrap_or_else(Vec::new).push(1);
```
```rust
let option = None::<Vec<u64>>;
option.unwrap_or_else(Vec::new).push(1);
```
(Aside: https://github.com/rust-lang/rust-clippy/pull/10120 unfairly contained multiple changes in one PR. I am trying to break that PR up into smaller pieces.)
---
changelog: FP: [`unwrap_or_else_default`]: No longer lints if the default value is needed for type inference
Rewrite [`tuple_array_conversions`]
Fixes#11100Fixes#11144Fixes#11124#11082 still needs discussion and #11085 likely can't be fixed.
changelog: [`tuple_array_conversions`]: Move to `pedantic`
changelog: [`tuple_array_conversions`]: Don't lint if mutability of references changes
changelog: [`tuple_array_conversions`]: Don't lint if bindings don't come from the exact same pattern
changelog: [`tuple_array_conversions`]: Don't lint if bindings are used for more than just the conversion
Add `imports_granularity = "Module"` to rustfmt.toml
This lets rustfmt split/merge imports, `Module` seems to be the most common style in clippy
https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#imports_granularity
changelog: none
Almost all the updates other than the config file change are from `cargo dev fmt` or blessed tests, the exceptions being
- `tests/ui/single_component_path_imports.rs`
- `tests/ui/single_component_path_imports_nested_first.rs`
- `tests/ui/single_component_path_imports_self_after.rs`
- `tests/ui/single_component_path_imports_self_before.rs`
- `tests/ui/unsafe_removed_from_name.rs` (added a test with merged imports as a drive by)
- `tests/ui/wildcard_imports.rs`
- `tests/ui/wildcard_imports_2021.rs`
[`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>`
Fixes#11145
This lint has a list of allowed types, one of which is `Wrapping<T>`, but it was only actually allowed if the type on the right hand side was also `Wrapping<T>`, which meant that, for example, `Wrapping<u32> += u32` would still lint. It now allows binary ops involving `Wrapping<T>` regardless of the type on the rhs.
These impls have only existed since Rust 1.60.0, so that is probably why the lint was previously not handling this correctly
changelog: [`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>` (e.g. `Wrapping<T> += T`)
This commit fixes#11025 by removing checks for `todo!`,
`unimplemented!` and `unreachable!`.
Signed-off-by: Panagiotis Foliadis <pfoliadis@hotmail.com>
Uplift `clippy::fn_null_check` lint
This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.
## `incorrect_fn_null_checks`
(warn-by-default)
The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.
### Example
```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */
if (fn_ptr as *const ()).is_null() { /* ... */ }
```
### Explanation
Function pointers are assumed to be non-null, checking for their nullity is incorrect.
-----
Mostly followed the instructions for uplifting a clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
`@rustbot` label: +I-lang-nominated
r? compiler
[`unnecessary_literal_unwrap`]: don't lint if binding initializer comes from expansion
Fixes https://github.com/rust-lang/rust-clippy/discussions/11109
changelog: [`unnecessary_literal_unwrap`]: don't lint if binding initializer comes from expansion
"try this" -> "try"
Current help messages contain a mix of "try", "try this", and one "try this instead". In the spirit of #10631, this PR adopts the first, as it is the most concise.
It also updates the `lint_message_conventions` test to catch cases of "try this".
(Aside: #10120 unfairly contained multiple changes in one PR. I am trying to break that PR up into smaller pieces.)
changelog: Make help messages more concise ("try this" -> "try").
Add `needless_pass_by_ref_mut` lint
changelog: [`needless_pass_by_ref_mut`]: This PR add a new lint `needless_pass_by_ref_mut` which emits a warning in case a `&mut` function argument isn't used mutably. It doesn't warn on trait and trait impls functions.
Fixes#8863.
cargo dev fmt
cargo test passes
cargo test passes
refactor a lil
Update bool_comparison.stderr
heavily refactor + bump `clippy::version`
refactor
refactor
check bounds to increase accuracy, and add todos
new lint: `read_line_without_trim`
This adds a new lint that checks for calls to `Stdin::read_line` with a reference to a string that is then attempted to parse into an integer type without first trimming it, which is always going to fail at runtime.
This is something that I've seen happen a lot to beginners, because it's easy to run into when following the example of chapter 2 in the book where it shows how to program a guessing game.
It would be nice if we could point beginners to clippy and tell them "let's see what clippy has to say" and have clippy explain to them why it fails 👀
I think this lint can later be "generalized" to work not just for `Stdin` but also any `BufRead` (which seems to be where the guarantee about the trailing newline comes from) and also, matching/comparing it to a string slice that doesn't end in a newline character (e.g. `input == "foo"` is always going to fail)
changelog: new lint: [`read_line_without_trim`]
[`useless_vec`]: add more tests and don't lint inside of macros
Closes#11084.
I realized that the fix I added in #11081 itself also causes an error in a suggestion when inside of a macro. Example:
```rs
macro_rules! x {
() => {
for _ in vec![1, 2] {}
}
}
x!();
```
Here it would suggest replacing `vec![1, 2]` with `[x!()]`, because that's what the source callsite is (reminder: it does this to get the correct span of `x!()` for code like `for _ in vec![x!()]`), but that's wrong when *inside* macros, so I decided to make it not lint if the whole loop construct is inside a macro to avoid this issue.
changelog: [`useless_vec`]: add more tests and don't lint inside of macros
r? `@Alexendoo` since these were your tests, I figured it makes most sense to assign you
Don't lint manual_let_else in cases where ? would work
Don't lint `manual_let_else` where the question mark operator `?` would be sufficient, that is, mostly in cases like:
```Rust
let v = if let Some(v) = ex { v } else { return None };
```
Also, this PR emits the `question_mark` lint for `let...else` patterns that could be written with `?` (also, only `return None` like cases).
```
changelog: [`manual_let_else`]: don't lint in cases where question_mark already lints
changelog: [`question_mark`]: lint for `let Some(...) = ex else { return None };`
```
Fixes #8755
[`useless_vec`]: use the source span for initializer
Fixes#11075.
changelog: [`useless_vec`]: use the source span for the initializer expression when inside of a macro
[`arc_with_non_send_sync`]: don't lint if type has nested type parameters
Fixes#11076
changelog: [`arc_with_non_send_sync`]: don't lint if type has nested type parameters
r? `@Manishearth`
new lint: `type_id_on_box`
Closes#7687.
A new lint that detects calling `.type_id()` on `Box<dyn Any>` (and not on the underlying `dyn Any`), which can make up for some pretty confusing bugs!
changelog: new lint: [`type_id_on_box`]
`let_and_return`: lint 'static lifetimes, don't lint borrows in closures
Fixes#11056
Now also ignores functions returning `'static` lifetimes, since I noticed the `stdin.lock()` example was still being linted but doesn't need to be since https://github.com/rust-lang/rust/pull/93965
changelog: none
