Emit `if_let_mutex` in presence of other mutexes
Currently (master, not nightly nor stable) `if_let_mutex` does not emit a warning here:
```rs
let m1 = Mutex::new(10);
let m2 = Mutex::new(());
if let 100..=200 = *m1.lock().unwrap() {
m2.lock();
} else {
m1.lock();
}
```
It currently looks for the first call to `.lock()` on *any* mutex receiver inside of the if/else body, and only later (outside of the visitor) checks that the receiver matches the mutex in the scrutinee. That means that in cases like the above, it finds the `m2.lock()` expression, stops the visitor, fails the check that it's the same mutex (`m2` != `m1`) and then does not look for any other `.lock()` calls.
So, just make the receiver check also part of the visitor so that we only stop the visitor when we also find the right receiver.
The first commit has the actual changes described here. The sceond one just unnests all the `if let`s
----
changelog: none
`missing_trait_methods`: lint methods in definition order
Lintcheck for #13157 showed a bunch of changes for `missing_trait_methods`
This is because `values_sorted` was sorting the entries by the key's [`DefPathHash`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/def_id/struct.DefPathHash.html), this is stable for a given compiler but can change across versions
changelog: none
Fix while_let_on_iterator dropping loop label when applying fix.
Loop label was not persisted when displaying help and was therefore producing broken rust code when applying fixes.
Solution was to store the `ast::Label` when creating a `higher::WhileLet` from an expression and add the label name to the lint suggestion and diagnostics.
---
Fixes: https://github.com/rust-lang/rust-clippy/issues/13123
changelog: [`while_let_on_iterator`]: Fix issue dropping loop label when displaying help and applying fixes.
Add `BTreeSet` detection to the `set_contains_or_insert` lint
* Detect `BTreeSet::contains` + `BTreeSet::insert` usage in the same way as with the `HashSet`.
CC: `@lochetti` `@bitfield`
----
changelog: [`set_contains_or_insert`]: Handle `BTreeSet` in addition to `HashSet`
Make `std_instead_of_core` somewhat MSRV aware
For #13158, this catches some things e.g. `core::net` and the recently stable `core::error` but not things moved individually like `UnwindSafe`, as far as I can see the version for those isn't easily available
Beta nominating since ideally we'd get this change in the same version as `core::error` becomes stable
cc `@kpreid`
changelog: none
needless_borrows_for_generic_args: Fix for &mut
This commit fixes a bug introduced in #12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, #12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.
Thus #12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.
However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.
To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.
Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.
Fixes#12856
changelog: none
Fix running compile-test under cargo nextest
`ui_test` itself has `cargo nextest` support which we now use - https://github.com/oli-obk/ui_test/pull/161
It prints `ui_test` as its test name whereas we printed `compile_test`, this ended up being treated as a test name filter causing all the tests to be filtered out
changelog: none
Misc refactorings part 5
`toplevel_ref_arg` gets a small fix so it can be allowed on function arguments. Otherwise just some rearrangements.
changelog: none
add lint for recreation of an entire struct
This lint makes Clippy warn about situations where an owned struct is
essentially recreated by moving all its fields into a new instance of
the struct. The lint is not machine-applicable because the source
struct may have been partially moved.
This lint originated in something I spotted during peer review. While
working on their branch a colleague ended up with a commit where a
function returned a struct that 1:1 replicated one of its owned inputs
from its members. Initially I suspected they hadn’t run their code
through Clippy but AFAICS there is no lint for this situation yet.
changelog: new lint: [`redundant_owned_struct_recreation`]
### New lint checklist
- \[+] Followed [lint naming conventions][lint_naming]
- \[+] Added passing UI tests (including committed `.stderr` file)
- \[+] `cargo test` passes locally
- \[+] Executed `cargo dev update_lints`
- \[+] Added lint documentation
- \[+] Run `cargo dev fmt`
[`pathbuf_init_then_push`]: Checks for calls to `push` immediately a…
changelog: [`pathbuf_init_then_push`]: new lint: Checks for calls to `push` immediately after creating a new `PathBuf`
Just a mirror of VEC_INIT_THEN_PUSH
Lint `zero_repeat_side_effects` only if array length is a literal zero
changelog: [`zero_repeat_side_effects` ] Lint only if array length is a literal zero
Fixes#13110 .
r? y21
Show progress while running dogfood test
Outputs the regular cargo progress in colour when running the dogfood test, helpful to see because it can take a long time to run
changelog: none
* Construct lint passes by taking `Conf` by reference.
* Use `HashSet` configs in less places
* Move some `check_crate` code into the pass constructor when possible.
This is a very large commit since a lot needs to be changed in order to
make the tests pass. The salient changes are:
- `ConstArgKind` gets a new `Path` variant, and all const params are now
represented using it. Non-param paths still use `ConstArgKind::Anon`
to prevent this change from getting too large, but they will soon use
the `Path` variant too.
- `ConstArg` gets a distinct `hir_id` field and its own variant in
`hir::Node`. This affected many parts of the compiler that expected
the parent of an `AnonConst` to be the containing context (e.g., an
array repeat expression). They have been changed to check the
"grandparent" where necessary.
- Some `ast::AnonConst`s now have their `DefId`s created in
rustc_ast_lowering rather than `DefCollector`. This is because in some
cases they will end up becoming a `ConstArgKind::Path` instead, which
has no `DefId`. We have to solve this in a hacky way where we guess
whether the `AnonConst` could end up as a path const since we can't
know for sure until after name resolution (`N` could refer to a free
const or a nullary struct). If it has no chance as being a const
param, then we create a `DefId` in `DefCollector` -- otherwise we
decide during ast_lowering. This will have to be updated once all path
consts use `ConstArgKind::Path`.
- We explicitly use `ConstArgHasType` for array lengths, rather than
implicitly relying on anon const type feeding -- this is due to the
addition of `ConstArgKind::Path`.
- Some tests have their outputs changed, but the changes are for the
most part minor (including removing duplicate or almost-duplicate
errors). One test now ICEs, but it is for an incomplete, unstable
feature and is now tracked at #127009.