Split `needless_borrow` into two lints
Splits off the case where the borrow is used as a generic argument to a function. I think the two cases are different enough to warrant a separate lint.
The tests for the new lint have been reordered to group related parts together. Two warning have been dropped, one looked like it was testing the generic argument form, but it ends up triggering the auto-deref variant. The second was just a redundant test that didn't do anything interesting.
An issue with cycle detection is also included. The old version was checking if a cycle was reachable from a block when it should have been checking if the block is part or a cycle.
As a side note, I'm liking the style of just jamming all the tests into separate scopes in main.
changelog: Split off `needless_borrows_for_generic_args` from `needless_borrow`
[`filter_map_bool_then`]: include multiple derefs from adjustments
In #11506 this lint was improved to suggest one deref if the bool is behind references (fixed the FP #11503), however it might need multiple dereferences if the bool is behind multiple layers of references or custom derefs. E.g. `&&&bool` needs `***b`.
changelog: [`filter_map_bool_then`]: suggest as many dereferences as there are needed to get to the bool
add extra `byref` checking for the guard's local
changelog: [`redundant_guards`]: Now checks if the variable is bound using `ref` before linting.
The lint should not be emitted, when the local variable is bind by-ref in the pattern.
fixes#11465
[`useless_conversion`]: don't lint if type parameter has unsatisfiable bounds for `.into_iter()` receiver
Fixes#11300.
Before this PR, clippy assumed that if it sees a `f(x.into_iter())` call and the type at that argument position is generic over any `IntoIterator`, then the `.into_iter()` call must be useless because `x` already implements `IntoIterator`, *however* this assumption is not right if the generic parameter has more than just the `IntoIterator` bound (because other traits can be implemented for the IntoIterator target type but not the IntoIterator implementor, as can be seen in the linked issue: `<[i32; 3] as IntoIterator>::IntoIter` satisfies `ExactSizeIterator`, but `[i32; 3]` does not).
So, this PR makes it check that the type parameter only has a single `IntoIterator` bound. It *might* be possible to check if the type of `x` in `f(x.into_iter())` satisfies all the bounds on the generic type parameter as defined on the function (which would allow removing the `.into_iter()` call even with multiple bounds), however I'm not sure how to do that, and the current fix should always work.
**Edit:** This PR has been changed to check if any of the bounds don't hold for the type of the `.into_iter()` receiver, so we can still lint in some cases.
changelog: [`useless_conversion`]: don't lint `.into_iter()` if type parameter has multiple bounds
new lint: `path_ends_with_ext`
Closes#11479
Not sure if it needs more test cases. I couldn't come up with any other ones, but it is a pretty simple lint logic wise with not too many checks
changelog: new lint: [`path_ends_with_ext`]
fix filter_map_bool_then with a bool reference
changelog: [`filter_map_bool_then`]: Fix the incorrect autofix when the `bool` in question is a reference.
fix#11503
[`extra_unused_type_parameters`]: Fix edge case FP for parameters in where bounds
Generic parameters can end up being used on the left side of where-bounds if they are not directly bound but instead appear nested in some concrete generic type. Therefore, we should walk the left side of where bounds, but only if the bounded type is *not* a generic param, in which case we still need to ignore the bound.
Fixes#11302
changelog: [`extra_unused_type_parameters`]: Fix edge case false positive for parameters in where bounds
Ignore closures for some type lints
Fixes#11417
`hir_ty_to_ty` is used in a couple of the `!is_local` lints, which doesn't play nicely inside bodies
changelog: none
Truncate files when opening in metadata-collector
Fixes the issue seen here https://github.com/rust-lang/rust-clippy/pull/11483#discussion_r1324687136 and in a couple other PRs
The changelog file was opened without truncating it, so if the new version is shorter than the old one stray contents would remain at the end of the file
The other two files first removed the file so didn't have this problem, but in all cases we now use `fs::write`/`File::create` which is write + create + truncate
changelog: none
[`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
Ignore span's parents in `collect_ast_format_args`/`find_format_args`
Fixes#11470, covers some cases missed by #10980
Can't have a test yet because of #11126 but it works locally
changelog: none
r? `@dswij`
new unnecessary_map_on_constructor lint
changelog: [`unnecessary_map_on_constructor`]: adds lint for cases in which map is not necessary. `Some(4).map(myfunction)` => `Some(myfunction(4))`
Closes https://github.com/rust-lang/rust-clippy/issues/6472
Note that the case mentioned in the issue `Some(..).and_then(|..| Some(..))` is fixed by a chain of lint changes. This PR completes the last part of that chain.
By `bind_instead_of_map`[lint](https://rust-lang.github.io/rust-clippy/master/index.html#/bind_instead_of_map):
`Some(4).and_then(|x| Some(foo(4)))` => `Some(4).map(|x| foo)`
By `redundant_closure` [lint](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure):
`Some(4).map(|x| foo)` => `Some(4).map(fun)`
Finally by this PR `unnecessary_map_on_constructor`:
`Some(4).map(fun)` => `Some(fun(4))`
I'm not sure this is the desired behavior for clippy and if it should be addressed in another issue/PR. I'd be up to give it a try if that's the case.
[`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
Fix `i686-unknown-linux-gnu` CI job
When testing https://github.com/oli-obk/ui_test/pull/161 I gave `--ignored` a try, I was surprised to see many of the 32bit tests passing even though I'm on a 64bit target
Turns out the `.stderr`s were incorrect, and our `i686-unknown-linux-gnu` job has been running `x86_64-unknown-linux-gnu` so it didn't get picked up
changelog: none
