Make `for_each_expr` visit closures by default, rename the old version `for_each_expr_without_closures`
A lot of the time `for_each_expr` is picked when closures should be visited so I think it makes sense for this to be the default with the alternative available for when you don't need to visit them.
The first commit renames `for_each_expr` to `for_each_expr_without_closures` and `for_each_expr_with_closures` to `for_each_expr`
The second commit switches a few uses that I caught over to include closures to fix a few bugs
changelog: none
Handle const effects inherited from parent correctly in `type_certainty`
This fixes a (debug) ICE in `type_certainty` that happened in the [k256 crate]. (I'm sure you can also specifically construct an edge test case that will run into type_certainty false positives visible outside of debug builds from this bug)
<details>
<summary>Minimal ICE repro</summary>
```rs
use std::ops::Add;
Add::add(1_i32, 1).add(i32::MIN);
```
</details>
The subtraction here overflowed:
436675b477/clippy_utils/src/ty/type_certainty/mod.rs (L209)
... when we have something like `Add::add` where `add` fn has 0 generic params but the `host_effect_index` is `Some(2)` (inherited from the parent generics, the const trait `Add`), and we end up executing `0 - 1`.
(Even if the own generics weren't empty and we didn't overflow, this would still be wrong because it would assume that a trait method with 1 generic parameter didn't have any generics).
So, *only* exclude the "host" generic parameter if it's actually bound by the own generics
changelog: none
[k256 crate]: https://github.com/RustCrypto/elliptic-curves/tree/master/k256
Revert: create const block bodies in typeck via query feeding
as per the discussion in https://github.com/rust-lang/rust/pull/125806#discussion_r1622563948
It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile.
reverts the const-block-specific parts of https://github.com/rust-lang/rust/pull/124650
`@bors` rollup=never had a small perf impact previously
fixes https://github.com/rust-lang/rust/issues/125846
r? `@compiler-errors`
Fix grammer for the Safety documentation check
The original message ("unsafe function's docs miss `# Safety` section") reads quite awkwardly. I've changed it to "unsafe function's docs are missing a `# Safety` section" to have it read better.
```
changelog: [`missing_headers`]: Tweak the grammar in the lint message
```
[`overly_complex_bool_expr`]: Fix trigger wrongly on never type
fixes#12689
---
changelog: fix [`overly_complex_bool_expr`] triggers wrongly on never type
Dedup nonminimal_bool_methods diags
Relates to #12379
Fix `nonminimal_bool` lint so that it doesn't check the same span multiple times.
`NotSimplificationVisitor` was called for each expression from `NonminimalBoolVisitor` whereas `NotSimplificationVisitor` also recursively checked all expressions.
---
changelog: [`nonminimal_bool`]: Fix duplicate diagnostics
The original message ("unsafe function's docs miss `# Safety` section")
reads quite awkwardly. I've changed it to "unsafe function's docs are missing
a `# Safety` section" to have it read better.
Signed-off-by: Paul R. Tagliamonte <paultag@gmail.com>
Modify str_to_string to be machine-applicable
Fixes https://github.com/rust-lang/rust-clippy/issues/12768
I'm not sure if there is any potential for edge cases with this - since it only ever acts on `&str` types I can't think of any, and especially since the methods do the same thing anyway.
changelog: allow `str_to_string` lint to be automatically applied
Disable `indexing_slicing` for custom Index impls
Fixes https://github.com/rust-lang/rust-clippy/issues/11525
Disables `indexing_slicing` for custom Index impls, specifically any implementations that also do not have a `get` method anywhere along the deref chain (so, for example, it still lints on Vec, which has its `get` method as part of the deref chain).
Thanks `@y21` for pointing me in the right direction with a couple of handy util functions for deref chain and inherent methods, saved a headache there!
changelog: FP: Disable `indexing_slicing` for custom Index impls
fix: let non_canonical_impls skip proc marco
Fixed#12788
Although the issue only mentions `NON_CANONICAL_CLONE_IMPL`, this fix will also affect `NON_CANONICAL_PARTIAL_ORD_IMPL` because I saw
> Because of these unforeseeable or unstable behaviors, macro expansion should often not be regarded as a part of the stable API.
on Clippy Documentation and these two lints are similar, so I think it might be good, not sure if it's right or not.
---
changelog: `NON_CANONICAL_CLONE_IMPL`, `NON_CANONICAL_PARTIAL_ORD_IMPL` will skip proc marco now
[`many_single_char_names`]: Deduplicate diagnostics
Relates to #12379
Fix `many_single_char_names` lint so that it doesn't emit diagnostics when the current level of the scope doesn't contain any single character name.
```rust
let (a, b, c, d): (i32, i32, i32, i32);
match 1 {
1 => (),
e => {},
}
```
produced the exact same MANY_SINGLE_CHAR_NAMES diagnostic at each of the Arm `e => {}` and the Block `{}`.
---
changelog: [`many_single_char_names`]: Fix duplicate diagnostics
Fix `unnecessary_to_owned` interaction with macro expansion
fixes#12821
In the case of an unnecessary `.iter().cloned()`, the lint `unnecessary_to_owned` might suggest to remove the `&` from references without checking if such references are inside a macro expansion. This can lead to unexpected behavior or even broken code if the lint suggestion is applied blindly. See issue #12821 for an example.
This PR checks if such references are inside macro expansions and skips this part of the lint suggestion in these cases.
changelog: [`unnecessary_to_owned`]: Don't suggest to remove `&` inside macro expansion
[perf] Delay the construction of early lint diag structs
Attacks some of the perf regressions from https://github.com/rust-lang/rust/pull/124417#issuecomment-2123700666.
See individual commits for details. The first three commits are not strictly necessary.
However, the 2nd one (06bc4fc67145e3a7be9b5a2cf2b5968cef36e587, *Remove `LintDiagnostic::msg`*) makes the main change way nicer to implement.
It's also pretty sweet on its own if I may say so myself.
`significant_drop_in_scrutinee`: Trigger lint only if lifetime allows early significant drop
I want to argue that the following code snippet should not trigger `significant_drop_in_scrutinee` (https://github.com/rust-lang/rust-clippy/issues/8987). The iterator holds a reference to the locked data, so it is expected that the mutex guard must be alive until the entire loop is finished.
```rust
use std::sync::Mutex;
fn main() {
let mutex_vec = Mutex::new(vec![1, 2, 3]);
for number in mutex_vec.lock().unwrap().iter() {
dbg!(number);
}
}
```
However, the lint should be triggered when we clone the vector. In this case, the iterator does not hold any reference to the locked data.
```diff
- for number in mutex_vec.lock().unwrap().iter() {
+ for number in mutex_vec.lock().unwrap().clone().iter() {
```
Unfortunately, it seems that regions on the types of local variables are mostly erased (`ReErased`) in the late lint pass. So it is hard to tell if the final expression has a lifetime relevant to the value with a significant drop.
In this PR, I try to make a best-effort guess based on the function signatures. To avoid false positives, no lint is issued if the result is uncertain. I'm not sure if this is acceptable or not, so any comments are welcome.
Fixes https://github.com/rust-lang/rust-clippy/issues/8987
changelog: [`significant_drop_in_scrutinee`]: Trigger lint only if lifetime allows early significant drop.
r? `@flip1995`
* instead simply set the primary message inside the lint decorator functions
* it used to be this way before [#]101986 which introduced `msg` to prevent
good path delayed bugs (which no longer exist) from firing under certain
circumstances when lints were suppressed / silenced
* this is no longer necessary for various reasons I presume
* it shaves off complexity and makes further changes easier to implement
fulfill expectations in `check_unsafe_derive_deserialize`
The utility function `clippy_utils::fulfill_or_allowed` is not used because using it would require to move the check for allowed after the check iterating over all inherent impls of the type, doing possibly unnecessary work.
Instead, `is_lint_allowed` is called as before, but additionally, once certain that the lint should be emitted, `span_lint_hir_and_then` is called instead of `span_lint_and_help` to also fulfill expectations.
Note: as this is my first contribution, please feel free to nitpick or request changes. I am happy to adjust the implementation.
fixes: #12802
changelog: fulfill expectations in [`unsafe_derive_deserialize`]
Add new lint `while_float`
This PR adds a nursery lint that checks for while loops comparing floating point values.
changelog:
```
changelog: [`while_float`]: Checks for while loops comparing floating point values.
```
Fixes#758
This avoids event spans that would otherwise cause crashes, since an
End's span covers the range of the tag (which will be earlier than the
line break within the tag).
Add configuration option for ignoring `panic!()` in tests
```
changelog: [`panic`]: Now can be disabled in tests with the `allow-panic-in-tests` option
```
I often find myself using `panic!(…)` in tests a lot, where I often do something like:
```rust
match enam {
Enam::A => …,
Enam::B => …,
_ => panic!("This should not happen at all."),
}
```
I think this patch should go nicely with already existing `allow-unwrap-in-tests` and `allow-expect-in-tests`.
make sure the msrv for `const_raw_ptr_deref` is met when linting [`missing_const_for_fn`]
fixes: #8864
---
changelog: make sure the msrv for `const_ptr_deref` is met when linting [`missing_const_for_fn`]
less aggressive needless_borrows_for_generic_args
Current implementation looks for significant drops, that can change the behavior, but that's not enough - value might not have a `Drop` itself but one of its children might have it.
A good example is passing a reference to `PathBuf` to `std::fs::File::open`. There's no benefits to pass `PathBuf` by value, but since `clippy` can't see `Drop` on `Vec` several layers down it complains forcing pass by value and making it impossible to use the same name later.
New implementation only looks at copy values or values created in place so existing variable will never be moved but things that take a string reference created and value is created inplace `&"".to_owned()` will make it to suggest to use `"".to_owned()` still.
Fixes https://github.com/rust-lang/rust-clippy/issues/12454
changelog: [`needless_borrows_for_generic_args`]: avoid moving variables
improve [`match_same_arms`] messages, enable rustfix test
closes: #9251
don't worry about the commit size, most of them are generated
---
changelog: improve [`match_same_arms`] lint messages
`significant_drop_in_scrutinee`: Fix false positives due to false drops of place expressions
Place expressions do not really create temporaries, so they will not create significant drops. For example, the following code snippet is quite good (#8963):
```rust
fn main() {
let x = std::sync::Mutex::new(vec![1, 2, 3]);
let x_guard = x.lock().unwrap();
match x_guard[0] {
1 => println!("1!"),
x => println!("{x}"),
}
drop(x_guard); // Some "usage"
}
```
Also, the previous logic thinks that references like `&MutexGuard<_>`/`Ref<'_, MutexGuard<'_, _>>` have significant drops, which is simply not true, so it is fixed together in this PR.
Fixes https://github.com/rust-lang/rust-clippy/issues/8963
Fixes https://github.com/rust-lang/rust-clippy/issues/9072
changelog: [`significant_drop_in_scrutinee`]: Fix false positives due to false drops of place expressions.
r? `@blyxyas`
add new lint that disallow renaming parameters in trait functions
fixes: #11443fixes: #486
changelog: add new lint [`renamed_function_params`]
Note that the lint name is not final, because I have a bad reputation in naming things, and I don't trust myself.
Lint direct priority conflicts in `[workspace.lints]`
Partially addresses #12729
This still doesn't do any workspace resolution stuff, so it will not catch any virtual workspaces or conflicts from inherited definitions. But while we're parsing the `Cargo.toml` we might as well check the workspace definitions if we find them
changelog: none
Handle `rustc_on_unimplemented` in duplicated_attributes
```rust
#[rustc_on_unimplemented(
on(
_Self = "&str",
label = "`a"
),
on(
_Self = "alloc::string::String",
label = "a"
),
)]
```
The lint treats this as a repetition because `rustc_on_unimplemented:🔛:label` appears twice, but that's ok.
Fixes#12619
changelog: [`duplicated_attributes`]: fix handling of `rustc_on_unimplemented`
Add new lint `doc_lazy_continuation`
changelog: [`doc_lazy_continuation`]: add lint that warns on so-called "lazy paragraph continuations"
This is a follow-up for https://github.com/rust-lang/rust/pull/121659, since most cases of unintended block quotes are lazy continuations. The lint is designed to be more generally useful than that, though, because it will also catch unintended list items and unintended block quotes that didn't coincidentally hit a pulldown-cmark bug.
The second commit is the result of running `cargo dev dogfood --fix`, and manually fixing anything that seems wrong. NOTE: this lint's suggestions should never change the parser's interpretation of the markdown, but in many cases, it seems that doc comments in clippy were written without regard for this feature of Markdown (which, I suppose, is why this lint should exist).
