Do not suggest `assigning_clones` in `Clone` impl
This PR modifies `assigning_clones` to detect situations where the `clone` call is inside a `Clone` impl, and avoids suggesting the lint in such situations.
r? `@blyxyas`
Fixes: https://github.com/rust-lang/rust-clippy/issues/12600
changelog: Do not invoke `assigning_clones` inside `Clone` impl
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local
Fixes#12616
`Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE
changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
Initialization expressions are now generated when index is bound to a variable.
FIX: Check to see if variables are used after swap
FIX: rename StmtKind::Local to StmtKind::Let
Elide unit variables linted by `let_unit` and use `()` directly instead
Situation: `let_unit` lints when an expression binds a unit (`()`) to a variable. In some cases this binding may be passed down to another function. Currently, the lint removes the binding without considering usage.
fixes: #12594
changelog: Suggestion Fix [`let_unit`]. Clippy will remove unit bindings and replace all their instances in the body with `()`.
Situation: `let_unit` lints when an expression binds a unit (`()`)
to a variable. In some cases this binding may be passed down to
another function. Currently, the lint removes the binding without
considering usage.
Change: All usages of the elided variable are now replaced with `()`.
fixes: #12594
`Box::default()` had its `#[rustc_box]` attribute removed in 1.69 so is
no longer a perf related lint
The lint is moved to style but no longer produces suggestions containing
turbofishes, as they're often longer/more annoying to type
Allow `filter_map_identity` when the closure is typed
This extends the `filter_map_identity` lint to support typed closures.
For untyped closures, we know that the program compiles, and therefore we can safely suggest using flatten.
For typed closures, they may participate in type resolution. In this case we use `Applicability::MaybeIncorrect`.
Details:
https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Should.20.60filter_map_identity.60.20lint.20when.20closures.20are.20typed.3F
changelog: `filter_map_identity` will now suggest using flatten for typed closures.
r? `@y21` && `@Centri3`
[`type_id_on_box`]: lint on any `Box<dyn _>`
Closes#11349.
It now not only lints when calling `.type_id()` on the type `Box<dyn Any>`, but also on any `Box<dyn Trait>` where `Trait` is a subtrait of `Any`
changelog: FN: [`type_id_on_box`]: lint if `Any` is a sub trait
new lint `legacy_numeric_constants`
Rework of #10997
- uses diagnostic items
- does not lint imports of the float modules (`use std::f32`)
- does not lint usage of float constants that look like `f32::MIN`
I chose to make the float changes because the following pattern is actually pretty useful
```rust
use std::f32;
let omega = freq * 2 * f32::consts::PI;
```
and the float modules are not TBD-deprecated like the integer modules.
Closes#10995
---
changelog: New lint [`legacy_numeric_constants`]
[#12312](https://github.com/rust-lang/rust-clippy/pull/12312)
make sure checked type implements `Try` trait when linting [`question_mark`]
(indirectly) fixes: #12412 and fixes: #11983
---
changelog: make sure checked type implements `Try` trait when linting [`question_mark`]
restrict manual_clamp to const case, bring it out of nursery
Implements the plan that I described in https://github.com/rust-lang/rust-clippy/pull/9484#issuecomment-1374522054
This does two things primarily
1. Restrict `manual_clamp` such that it will only trigger if we are able to guarantee that `clamp` won't panic at runtime.
2. Bring `manual_clamp` out of nursery status and move it into the complexity group.
changelog: [`manual_clamp`]: Restrict this lint such that it only triggers if max and min are const, and max is greater than or equal to min. Then bring it out of the nursery group.
Fix typo in comment
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
`changelog: none`. Otherwise, please write a short comment
explaining your change.
It's also helpful for us that the lint name is put within backticks (`` ` ` ``),
and then encapsulated by square brackets (`[]`), for example:
```
changelog: [`lint_name`]: your change
```
If your PR fixes an issue, you can add `fixes #issue_number` into this
PR description. This way the issue will be automatically closed when
your PR is merged.
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]
- \[ ] Added passing UI tests (including committed `.stderr` file)
- \[ ] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[ ] Added lint documentation
- \[x] Run `cargo dev fmt`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR.
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: None
Instead of just saying “this function's stack frame is big”, report:
* the (presumed) size of the frame
* the size and type of the largest local contributing to that size
* the configurable limit that was exceeded (once)
allow [`manual_unwrap_or_default`] in const function
closes: #12568
---
changelog: allow [`manual_unwrap_or_default`] in const function
This is a small fix, I was originally decided to fix it along with `#12568` but there are some problems needs to be addressed (which is why my branch is called `issue12569` 😆 ), so I decide to open a separated PR to fix them one at a time.
Rename `Inherited` -> `TypeckRootCtxt`
`Inherited` is a confusing name. Rename it to `TypeckRootCtxt`.
I don't think this needs a type MCP or anything since it's not nearly as pervasive as `FnCtxt` , for example.
r? `@lcnr` `@oli-obk`
It turns out there is a bit of a circular dependency - I cannot add
anything to `core` because Clippy fails, and I can't actually add
correct Clippy implementations without new implementations from `core`.
Change some of the Clippy stubs from `unimplemented!` to success values
and leave a FIXME in their place to mitigate this.
Fixes <https://github.com/rust-lang/rust/issues/122587>
Change applicability of `assigning_clones` to `Unspecified`
Before we deal with https://github.com/rust-lang/rust-clippy/pull/12473 and the borrow checker errors, I think that it would be better to downgrade this lint, since it can break code.
changelog: Change the applicability of `assigning_clones` to `Unspecified`
r? `@blyxyas`
fix: `suspicious_else_formatting` false positive when else is included …
This PR addresses an issue where invalid suggestions are generated for `if-else` formatting if comments contain the keyword `else`.
The root of the problem is identified [here](95c62ffae9/clippy_lints/src/formatting.rs (L217)). Specifically, when a comment contains the word `else`, the lint mistakenly interprets it as part of an `if-else` clause. This misinterpretation leads to an incorrect splitting of the snippet, resulting in erroneous suggestions.
fixes: #12497
changelog: [`suspicious_else_formatting`]: Fixes invalid suggestions when comments include word else
Rename `hir::Local` into `hir::LetStmt`
Follow-up of #122776.
As discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Improve.20naming.20of.20.60ExprKind.3A.3ALet.60.3F).
I made this change into a separate PR because I'm less sure about this change as is. For example, we have `visit_local` and `LocalSource` items. Is it fine to keep these two as is (I supposed it is but I prefer to ask) or not? Having `Node::Local(LetStmt)` makes things more explicit but is it going too far?
r? ```@oli-obk```
`useless_asref`: do not lint `.as_ref().map(Arc::clone)`
This applies to `Arc`, `Rc`, and their weak variants. Using `.clone()` would be less idiomatic.
This follows the discussion in <https://github.com/rust-lang/rust-clippy/issues/12528#issuecomment-2014444305>.
changelog: [`useless_asref`]: do not lint `.as_ref().map(Arc::clone)` and similar
don't lint [`mixed_attributes_style`] when mixing docs and other attrs
fixes: #12435fixes: #12436fixes: #12530
---
changelog: don't lint [`mixed_attributes_style`] when mixing different kind of attrs; and move it to late pass;
don't lint [`mixed_attributes_style`] when mixing docs and other attrs
add test files for issue #12436
move [`mixed_attributes_style`] to `LateLintPass` to enable global `allow`
stop [`mixed_attributes_style`] from linting on different attributes
add `@compile-flags` to [`mixed_attributes_style`]'s test;
turns out not linting in test mod is not a FN.
Apply suggestions from code review
Co-authored-by: Timo <30553356+y21@users.noreply.github.com>
move [`mixed_attributes_style`] to late pass and stop it from linting on different kind of attributes
Rollup of 8 pull requests
Successful merges:
- #114009 (compiler: allow transmute of ZST arrays with generics)
- #122195 (Note that the caller chooses a type for type param)
- #122651 (Suggest `_` for missing generic arguments in turbofish)
- #122784 (Add `tag_for_variant` query)
- #122839 (Split out `PredicatePolarity` from `ImplPolarity`)
- #122873 (Merge my contributor emails into one using mailmap)
- #122885 (Adjust better spastorino membership to triagebot's adhoc_groups)
- #122888 (add a couple more tests)
r? `@ghost`
`@rustbot` modify labels: rollup
Do not warn on .map(_::clone) for Arc, Rc, and their weak variants
Those constructions are idiomatic, and using `Arc::clone(x)` and `Rc::clone(x)` is often the recommended way of cloning a `Arc` or a `Rc`.
Fix#12528
changelog: [`map_clone`]: do not warn on `.map(_::clone)` for `Arc`, `Rc`, and their `Weak` variants
Fix infinite loop in `cast_sign_loss` when peeling unwrap method calls
Fixes#12506
The lint wants to peel method calls but didn't actually reassign the expression, leading to an infinite loop.
----
changelog: Fix infinite loop in [`cast_sign_loss`] when having two chained `.unwrap()` calls
Mention `size_hint()` effect in `flat_map_option` lint documentation.
The previous documentation for `flat_map_option` mentioned only readability benefits, but there is also at least one performance benefit: the `size_hint()` upper bound is preserved, whereas `flat_map().size_hint()` is always `(0, None)`.
Program demonstrating this difference:
```rust
fn main() {
let evens = |i| if i % 2 == 0 { Some(i) } else { None };
dbg!(
[1, 2, 3].iter().flat_map(evens).size_hint(),
[1, 2, 3].iter().filter_map(evens).size_hint(),
);
}
```
changelog: [`flat_map_option`]: Mention the benefit to `size_hint()`.
Experimental feature postfix match
This has a basic experimental implementation for the RFC postfix match (rust-lang/rfcs#3295, #121618). [Liaison is](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Postfix.20Match.20Liaison/near/423301844) ```@scottmcm``` with the lang team's [experimental feature gate process](https://github.com/rust-lang/lang-team/blob/master/src/how_to/experiment.md).
This feature has had an RFC for a while, and there has been discussion on it for a while. It would probably be valuable to see it out in the field rather than continue discussing it. This feature also allows to see how popular postfix expressions like this are for the postfix macros RFC, as those will take more time to implement.
It is entirely implemented in the parser, so it should be relatively easy to remove if needed.
This PR is split in to 5 commits to ease review.
1. The implementation of the feature & gating.
2. Add a MatchKind field, fix uses, fix pretty.
3. Basic rustfmt impl, as rustfmt crashes upon seeing this syntax without a fix.
4. Add new MatchSource to HIR for Clippy & other HIR consumers
Several (doc) comments were super outdated or didn't provide enough context.
Some doc comments shoved everything in a single paragraph without respecting
the fact that the first paragraph should be a single sentence because rustdoc
treats these as item descriptions / synopses on module pages.
Split an item bounds and an item's super predicates
This is the moral equivalent of #107614, but instead for predicates this applies to **item bounds**. This PR splits out the item bounds (i.e. *all* predicates that are assumed to hold for the alias) from the item *super predicates*, which are the subset of item bounds which share the same self type as the alias.
## Why?
Much like #107614, there are places in the compiler where we *only* care about super-predicates, and considering predicates that possibly don't have anything to do with the alias is problematic. This includes things like closure signature inference (which is at its core searching for `Self: Fn(..)` style bounds), but also lints like `#[must_use]`, error reporting for aliases, computing type outlives predicates.
Even in cases where considering all of the `item_bounds` doesn't lead to bugs, unnecessarily considering irrelevant bounds does lead to a regression (#121121) due to doing extra work in the solver.
## Example 1 - Trait Aliases
This is best explored via an example:
```
type TAIT<T> = impl TraitAlias<T>;
trait TraitAlias<T> = A + B where T: C;
```
The item bounds list for `Tait<T>` will include:
* `Tait<T>: A`
* `Tait<T>: B`
* `T: C`
While `item_super_predicates` query will include just the first two predicates.
Side-note: You may wonder why `T: C` is included in the item bounds for `TAIT`? This is because when we elaborate `TraitAlias<T>`, we will also elaborate all the predicates on the trait.
## Example 2 - Associated Type Bounds
```
type TAIT<T> = impl Iterator<Item: A>;
```
The `item_bounds` list for `TAIT<T>` will include:
* `Tait<T>: Iterator`
* `<Tait<T> as Iterator>::Item: A`
But the `item_super_predicates` will just include the first bound, since that's the only bound that is relevant to the *alias* itself.
## So what
This leads to some diagnostics duplication just like #107614, but none of it will be user-facing. We only see it in the UI test suite because we explicitly disable diagnostic deduplication.
Regarding naming, I went with `super_predicates` kind of arbitrarily; this can easily be changed, but I'd consider better names as long as we don't block this PR in perpetuity.
Changelog for Clippy 1.77 🏫
Roses are violets,
Red is blue,
Let's create a world,
Perfect for me and you
---
### The cat of this release is: *Luigi*
<img width=500 src="https://github.com/rust-lang/rust-clippy/assets/17087237/ea13d05c-e5ba-4189-9e16-49bf1b43c468" alt="The cats of this Clippy release" />
The cat for the next release can be voted on: [here](https://forms.gle/57gbrNvXtCUmrHYh6)
The cat for the next next release can be nominated in the comments and will be voted in the next changelog PR (Submission deadline is 2024-03-30 23:59CET)
---
changelog: none
Disable `cast_lossless` when casting to u128 from any (u)int type
Fixes https://github.com/rust-lang/rust-clippy/issues/12492
Disables `cast_lossless` when casting to u128 from any int or uint type. The lint states that when casting to any int type, there can potentially be lossy behaviour if the source type ever exceeds the size of the destination type in the future, which is impossible with a destination of u128.
It's possible this is a bit of a niche edge case which is better addressed by just disabling the lint in code, but I personally couldn't think of any good reason to still lint in this specific case - maybe except if the source is a bool, for readability reasons :).
changelog: FP: `cast_lossless`: disable lint when casting to u128 from any (u)int type
Make `assigning_clones` MSRV check more precise
Continuation of #12511
`clone_into` is the only suggestion subject to the 1.63 MSRV requirement, and the lint should still emit other suggestions regardless of the MSRV.
changelog: [assigning_clones]: only apply MSRV check to `clone_into` suggestions.
`assigning_clones` should respect MSRV
Fixes: #12502
This PR fixes the `assigning_clones` lint suggesting to use `clone_from` or `clone_into` on incompatible MSRVs.
`assigning_clones` will suggest using either `clone_from` or `clone_into`, both of which were stabilized in 1.63. If the current MSRV is below 1.63, the lint should not trigger.
changelog: [`assigning_clones`]: don't lint when the MSRV is below 1.63.
Use hir::Node helper methods instead of repeating the same impl multiple times
I wanted to do something entirely different and stumbled upon a bunch of cleanups
[`map_entry`]: call the visitor on the local's `else` block
Fixes#12489
The lint already has all the logic it needs for figuring out if it can or can't suggest a closure if it sees control flow expressions like `break` or `continue`, but it was ignoring the local's else block, which meant that it didn't see the `return None;` in a `let..else`.
changelog: [`map_entry`]: suggest `if let` instead of a closure when `return` expressions exist in the else block of a `let..else`
new restriction lint: `integer_division_remainder_used`
Fixes https://github.com/rust-lang/rust-clippy/issues/12391
Introduces a restriction lint which disallows the use of `/` and `%` operators on any `Int` or `Uint` types (i.e., any that use the default `Div` or `Rem` trait implementations). Custom implementations of these traits are ignored.
----
changelog: Add new restriction lint [`integer_division_remainder_used`]
[`option_option`]: Fix duplicate diagnostics
Relates to #12379
This `option_option` lint change skips checks against `ty`s inside `field_def`s defined by external macro to prevent duplicate diagnostics to the same span `ty` by multiple `Struct` definitions.
---
changelog: [`option_option`]: Fix duplicate diagnostics
move `readonly_write_lock` to perf
[There haven't been any issues](https://github.com/rust-lang/rust-clippy/issues?q=is%3Aissue+readonly_write_lock) since its creation and it's a pretty useful lint I think, so I'd say it's worth giving it a try?
Did a lintcheck run on 300 crates with no results, but I guess `RwLock` is usually not something that's used much in libraries.
changelog: move [`readonly_write_lock`] to perf (now warn-by-default)
fix [`dbg_macro`] FN when dbg is inside some complex macros
fixes: #12131
It appears that [`root_macro_call_first_node`] only detects `println!` in the following example:
```rust
println!("{:?}", dbg!(s));
```
---
changelog: fix [`dbg_macro`] FN when `dbg` is inside some complex macros
(re-opening b'cuz bors doesn't like my previous one)
fix span calculation for non-ascii in `needless_return`
Fixes#12491
Probably fixes#12328 as well, but that one has no reproducer, so 🤷
The bug was that the lint used `rfind()` for finding the byte index of the start of the previous non-whitespace character:
```
// abc\n return;
^
```
... then subtracting one to get the byte index of the actual whitespace (the `\n` here).
(Subtracting instead of adding because it treats this as the length from the `return` token to the `\n`)
That's correct for ascii, like here, and will get us to the `\n`, however for non ascii, the `c` could be multiple bytes wide, which would put us in the middle of a codepoint if we simply subtract 1 and is what caused the ICE.
There's probably a lot of ways we could fix this.
This PR changes it to iterate backwards using bytes instead of characters, so that when `rposition()` finally finds a non-whitespace byte, we *know* that we've skipped exactly 1 byte. This was *probably*(?) what the code was intending to do
changelog: Fix ICE in [`needless_return`] when previous line end in a non-ascii character
hir: Remove `opt_local_def_id_to_hir_id` and `opt_hir_node_by_def_id`
Also replace a few `hir_node()` calls with `hir_node_by_def_id()`.
Follow up to https://github.com/rust-lang/rust/pull/120943.
[`unused_enumerate_index`]: trigger on method calls
The lint used to check for patterns looking like:
```rs
for (_, x) in some_iter.enumerate() {
// Index is ignored
}
```
This commit further checks for chained method calls constructs where we
can detect that the index is unused. Currently, this checks only for the
following patterns:
```rs
some_iter.enumerate().map_function(|(_, x)| ..)
let x = some_iter.enumerate();
x.map_function(|(_, x)| ..)
```
where `map_function` is one of `all`, `any`, `filter_map`, `find_map`,
`flat_map`, `for_each` or `map`.
Fixes#12411.
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`unused_enumerate_index`]: add detection for method chains such as `iter.enumerate().map(|(_, x)| x)`
Prior to this change, it might be that the lint would remove an explicit
type that was necessary for the type system to keep track of types.
This should be the last change that prevented this lint to be machine
applicable.
Don't emit `doc_markdown` lint for missing backticks if it's inside a quote
Fixes#10262.
changelog: Don't emit `doc_markdown` lint for missing backticks if it's inside a quote
[`use_self`]: Make it aware of lifetimes
Have the lint trigger even if `Self` has generic lifetime parameters.
```rs
impl<'a> Foo<'a> {
type Item = Foo<'a>; // Can be replaced with Self
fn new() -> Self {
Foo { // No lifetime, but they are inferred to be that of Self
// Can be replaced as well
...
}
}
// Don't replace `Foo<'b>`, the lifetime is different!
fn eq<'b>(self, other: Foo<'b>) -> bool {
..
}
```
Fixes#12381
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`use_self`]: Have the lint trigger even if `Self` has generic lifetime parameters
The lint used to check for patterns looking like:
```rs
for (_, x) in some_iter.enumerate() {
// Index is ignored
}
```
This commit further checks for chained method calls constructs where we
can detect that the index is unused. Currently, this checks only for the
following patterns:
```rs
some_iter.enumerate().map_function(|(_, x)| ..)
let x = some_iter.enumerate();
x.map_function(|(_, x)| ..)
```
where `map_function` is one of `all`, `any`, `filter_map`, `find_map`,
`flat_map`, `for_each` or `map`.
Fixes#12411.
lint when calling the blanket `Into` impl from a `From` impl
Closes#11150
```
warning: function cannot return without recursing
--> x.rs:9:9
|
9 | / fn from(value: f32) -> Self {
10 | | value.into()
11 | | }
| |_________^
|
note: recursive call site
--> x.rs:10:13
|
10 | value.into()
| ^^^^^^^^^^^^
```
I'm also thinking that we can probably generalize this lint to #11032 at some point (instead of hardcoding a bunch of impls), like how rustc's `unconditional_recursion` works, at least up to one indirect call, but this still seems useful for now :)
I've also noticed that we use `fn_def_id` in a bunch of lints and then try to get the node args of the call in a separate step, so I made a helper function that does both in one. I intend to refactor a bunch of uses of `fn_def_id` to use this later
I can add more test cases, but this is already using much of the same logic that exists for the other impls that this lint looks for (e.g. making sure that there are no conditional returns).
changelog: [`unconditional_recursion`]: emit a warning inside of `From::from` when unconditionally calling the blanket `.into()` impl
chore: fix some typos
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
`changelog: none`. Otherwise, please write a short comment
explaining your change.
It's also helpful for us that the lint name is put within backticks (`` ` ` ``),
and then encapsulated by square brackets (`[]`), for example:
```
changelog: [`lint_name`]: your change
```
If your PR fixes an issue, you can add `fixes #issue_number` into this
PR description. This way the issue will be automatically closed when
your PR is merged.
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]
- \[ ] Added passing UI tests (including committed `.stderr` file)
- \[ ] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[ ] Added lint documentation
- \[x] Run `cargo dev fmt`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR.
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog:
