Fix `mut_mutex_lock` when Mutex is behind immutable deref
I *think* the problem here is the `if let ty::Ref(_, _, Mutability::Mut) = cx.typeck_results().expr_ty(recv).kind()` line tries to check if the `Mutex` can be mutably borrowed (there already is a test for `Arc<Mutex<_>>`), but gets bamboozled by the `&mut Arc` indirection. And I *think* checking the deref-adjustment to filter immutable-adjust (the deref through the `Arc`, starting from `&mut Arc`) is the correct fix.
Fixes#9415
changelog: Fix `mut_mutex_lock` when Mutex is behind immutable deref
Don't use `hir_ty_to_ty` in `result_large_err`
fixes#9414
This occurs starting with 2022-09-01. I checked that this does fix the ICE on rust-lang/rust@9353538. Not sure which pr caused the late-bound region to leak through `hir_ty_to_ty`.
changelog: None
Fix `suboptimal_float` not linting on `{const}.powf({const})`
There used to be an early return if the receiver was an effective const but the method was not linted, not taking into account later cases where the receiver and the arguments are both effective consts for different methods. Removed the early return.
Fixes#9402Fixes#9201
changelog: Fix `suboptimal_flops`, `imprecise_flops` not linting on `{const}.powf({const})` et al
Fix the emission order of `trait_duplication_in_bounds`
Makes the lint emit in source order rather than whatever order the hash map happens to be in. This is currently blocking the sync into rustc.
changelog: None
Strengthen invalid_value lint to forbid uninit primitives, adjust docs to say that's UB
For context: https://github.com/rust-lang/rust/issues/66151#issuecomment-1174477404=
This does not make it a FCW, but it does explicitly state in the docs that uninit integers are UB.
This also doesn't affect any runtime behavior, uninit u32's will still successfully be created through mem::uninitialized.
Fix missing parens in `suboptimal_flops` suggestion
Fixes#9391. The problem is simple enough, I didn't check if the same problem occurs elsewhere, though.
changelog: fix missing parenthesis in `suboptimal_flops` suggestion
Ignore `match_like_matches_macro` when there is comment
Closes#9164
changelog: [`match_like_matches_macro`] is ignored when there is some comment inside the match block.
Also add `span_contains_comment` util to check if given span contains comments.
Implemented `suspicious_to_owned` lint to check if `to_owned` is called on a `Cow`
changelog: Add lint ``[`suspicious_to_owned`]``
-----------------
Hi,
posting this unsolicited PR as I've been burned by this issue :)
Being unsolicited, feel free to reject it or reassign a different lint level etc.
This lint checks whether `to_owned` is called on `Cow<'_, _>`. This is done because `to_owned` is very similarly named to `into_owned`, but the effect of calling those two methods is completely different (one makes the `Cow::Borrowed` into a `Cow::Owned`, the other just clones the `Cow`). If the cow is then passed to code for which the type is not checked (e.g. generics, closures, etc.) it might slip through and if the cow data is coming from an unsafe context there is the potential for accidentally cause undefined behavior.
Even if not falling into this painful case, there's really no reason to call `to_owned` on a `Cow` other than confusing people reading the code: either `into_owned` or `clone` should be called.
Note that this overlaps perfectly with `implicit_clone` as a warning, but `implicit_clone` is classified pedantic (while the consequences for `Cow` might be of a wider blast radius than just pedantry); given the overlap, I set-up the lint so that if `suspicious_to_owned` triggers `implicit_clone` will not trigger. I'm not 100% sure this is done in the correct way (I tried to copy what other lints were doing) so please provide feedback on it if it isn't.
### Checklist
- \[x] Followed [lint naming conventions][lint_naming]
- \[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`
This is done because `to_owned` is very similarly named to `into_owned`, but the
effect of calling those two methods is completely different. This creates
confusion (stemming from the ambiguity of the 'owned' term in the context of
`Cow`s) and might not be what the writer intended.
new lint
This fixes#6576
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[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`
---
changelog: add [`multi_assignments`] lint
feat(fix): Do not lint if the target code is inside a loop
close#8753
we consider the following code.
```rust
fn main() {
let vec = vec![1];
let w: Vec<usize> = vec.iter().map(|i| i * i).collect(); // <- once.
for i in 0..2 {
let _ = w.contains(&i);
}
}
```
and the clippy will issue the following warning.
```rust
warning: avoid using `collect()` when not needed
--> src/main.rs:3:51
|
3 | let w: Vec<usize> = vec.iter().map(|i| i * i).collect();
| ^^^^^^^
...
6 | let _ = w.contains(&i);
| -------------- the iterator could be used here instead
|
= note: `#[warn(clippy::needless_collect)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
help: check if the original Iterator contains an element instead of collecting then checking
|
3 ~
4 |
5 | for i in 0..2 {
6 ~ let _ = vec.iter().map(|i| i * i).any(|x| x == i);
```
Rewrite the code as indicated.
```rust
fn main() {
let vec = vec![1];
for i in 0..2 {
let _ = vec.iter().map(|i| i * i).any(|x| x == i); // <- execute `map` every loop.
}
}
```
this code is valid in the compiler, but, it is different from the code before the rewrite.
So, we should not lint, If `collect` is outside of a loop.
Thank you in advance.
---
changelog: Do not lint if the target code is inside a loop in `needless_collect`
Lint `collapsible_str_replace`
fixes#6651
```
changelog: [`collapsible_str_replace`]: create new lint `collapsible_str_replace`
```
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
Rework `only_used_in_recursion`
fixes#8782fixes#8629fixes#8560fixes#8556
This is a complete rewrite of the lint. This loses some capabilities of the old implementation. Namely the ability to track through tuple and slice patterns, as well as the ability to trace through assignments.
The two reported bugs are fixed with this. One was caused by using the name of the method rather than resolving to the `DefId` of the called method. The second was cause by using the existence of a cycle in the dependency graph to determine whether the parameter was used in recursion even though there were other ways to create a cycle in the graph.
Implementation wise this switches from using a visitor to walking up the tree from every use of each parameter until it has been determined the parameter is used for something other than recursion. This is likely to perform better as it avoids walking the entire function a second time, and it is unlikely to walk up the HIR tree very much. Some cases would perform worse though.
cc `@buttercrab`
changelog: Scale back `only_used_in_recursion` to fix false positives
changelog: Move `only_used_in_recursion` back to `complexity`
Enhance `needless_borrow` to consider trait implementations
The proposed enhancement causes `needless_borrow` to suggest removing `&` from `&e` when `&e` is an argument position requiring trait implementations, and `e` implements the required traits. Example:
```
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:131:51
|
LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
| ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
```
r? `@Jarcho`
changelog: Enhance `needless_borrow` to consider trait implementations
unwrap_used and expect_used: trigger on uses of their _err variants
changelog: [`unwrap_used`]: lint uses of `unwrap_err`
changelog: [`expect_used`]: lint uses of `expect_err`
fixes#9331
`transmute_undefined_repr` fix
changelog: Don't lint `transmute_undefined_repr` when the the first field of a `repr(C)` type is compatible with the other type
Use `CARGO_TARGET_DIR` in compile-test
changelog: none
I have a global `CARGO_TARGET_DIR` set, but forgot to delete the old target dir. `compile-test` was getting tripped up on an outdated `rustfix_missing_coverage.txt` I had in there, keeping me from running tests 😄.
Fix [`non_ascii_literal`] in tests
changelog: Don't lint [`non_ascii_literal`] when using non-ascii comments in tests
changelog: Don't lint [`non_ascii_literal`] when `allow`ed on tests
closes: #7739closes: #8263
Add new lint [`positional_named_format_parameters`]
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Add new lint [`positional_named_format_parameters`] to warn when named parameters in format strings are used as positional arguments.
Fix if_let_mutex not checking Mutexes behind refs
Fixes#9193
We can always peel references because we are looking for a method-call, for which autoderef applies.
---
changelog: [`if_let_mutex`]: detect calls to `Mutex::lock()` if mutex is behind a ref
changelog: [`if_let_mutex`]: Add labels to the two instances of the same Mutex that will deadlock
Add lint recommending using `std::iter::once` and `std::iter::empty`
```
changelog: [`iter_once`]: add new lint
changelog: [`iter_empty`]: add new lint
```
fixes#9186
- \[ ] Followed [lint naming conventions][lint_naming]
- \[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`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
The lint doesn't really follow the naming conventions. I don't have any better idea so I'm open to suggestions.
Extend `if_then_some_else_none` to also suggest `bool::then_some`
Closes#9094.
changelog: Extend `if_then_some_else_none` to also suggest `bool::then_some`
Add partialeq_to_none lint
Initial implementation of #9275, adding lint `partialeq_to_none`. This is my first time working on `clippy`, so please review carefully.
I'm unsure especially about the `Sugg`, as it covers the entire `BinOp`, instead of just covering one of the sides and the operator (see the multi-line example). I was unsure if pinpointing the suggestion wouldn't be brittle...
changelog: [`PARTIALEQ_TO_NONE`]: Initial commit
`explicit_auto_deref` changes
fixes#9123fixes#9109fixes#9143fixes#9101
This avoid suggesting code which hits a rustc bug. Basically `&{x}` won't use auto-deref if the target type is `Sized`.
changelog: Don't suggest using auto deref for block expressions when the target type is `Sized`
changelog: Include the borrow in the suggestion for `explicit_auto_deref`
changelog: Don't lint `explicit_auto_deref` on `dyn Trait` return
changelog: Don't lint `explicit_auto_deref` when other adjustments are required
changelog: Lint `explicit_auto_deref` in implicit return positions for closures
Fix suggestions for `async` closures in redundant_closure_call
Fixes#9052
changelog: Fix suggestions given by [`redundant_closure_call`] for async closures
- only compare where predicates to trait bounds when generating where
clause specific message to fix#9151
- use comparable_trait_ref to account for trait bound generics to fix#8757
Move [`assertions_on_result_states`] to restriction
Close#9263
This lint causes regression on readability of code and log output. And printing runtime values is not particularly useful for majority of tests which should be reproducible.
changelog: Move [`assertions_on_result_states`] to restriction and don't lint it for unit type
Signed-off-by: tabokie <xy.tao@outlook.com>
unwrap_used: Don't recommend using `expect` when the `expect_used` lint is not allowed
Fixes#9222
```
changelog: [`unwrap_used`]: Don't recommend using `expect` when the `expect_used` lint is not allowed
```
Read and use deprecated configuration (as well as emitting a warning)
Original change written by `@flip1995` I've simply rebased to master and fixed up the formatting/tests. This change teaches the configuration parser which config key replaced a deprecated key and attempts to populate the latter from the former. If both keys are provided this fails with a duplicate key error (rather than attempting to guess which the user intended).
Currently this on affects `cyclomatic-complexity-threshold` -> `cognitive-complexity-threshold` but will also be used in #8974 to handle `blacklisted-names` -> `disallowed-names`.
```
changelog: deprecated configuration keys are still applied as if they were provided as their non-deprecated name.
```
- [x] `cargo test` passes locally
- [x] Run `cargo dev fmt`
