Simplify `use crate::rustc_foo::bar` occurrences.
They can just be written as `use rustc_foo::bar`, which is far more standard. (I didn't even know that a `crate::` prefix was valid.)
r? ``@eholk``
Remove braces when fixing a nested use tree into a single item
[Back in 2019](https://github.com/rust-lang/rust/pull/56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`.
This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then.
A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`.
This PR is best reviewed commit-by-commit.
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.
Allow more attributes in `clippy::useless_attribute`
Fixes#12753Fixes#4467Fixes#11595Fixes#10878
changelog: [`useless_attribute`]: Attributes allowed on `use` items now include `ambiguous_glob_exports`, `hidden_glob_reexports`, `dead_code`, `unused_braces`, and `clippy::disallowed_types`.
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 inplace
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
Fix `FormatArgs` storage when `-Zthreads` > 1
Fixes#11886
The initial way I thought of was a little gross so I never opened a PR for it, I thought of a nicer way today that no longer involves any `thread_local`s or `static`s
`rustc_data_strucutres::sync::{Lrc, OnceLock}` implement `DynSend` + `DynSync` so we can pass them to the lint passes that need the storage
changelog: none
r? `@flip1995`
Suggest collapsing nested or patterns if the MSRV allows it
Nested `or` patterns have been stable since 1.53, so we should be able to suggest `Some(1 | 2)` if the MSRV isn't set below that.
This change adds an msrv check and also moves it to `matches/mod.rs`, because it's also needed by `redundant_guards`.
changelog: [`collapsible_match`]: suggest collapsing nested or patterns if the MSRV allows it
Changelog for Clippy 1.78 🪄
Roses and Violets have colors,
Red and Blue are the two,
I'm getting to the end of my masters,
what a cool goal to pursue
---
### The cat of this release is: *Shadow* submitted by `@benwh1:`
<img height=500 src="https://github.com/rust-lang/rust-clippy/assets/32911992/c56af314-4644-482a-a08e-f32f4c7d7b22" alt="The cats of this Clippy release" />
Cats for the next release can be nominated in the comments :D
---
changelog: none
fix suggestion error for [`manual_is_ascii_check`] with missing type
fixes: #11324fixes: #11776
changelog: improve [`manual_is_ascii_check`] to suggest labeling type in closure, fix FP with type generics, and improve linting on ref expressions.
suppress `readonly_write_lock` for underscore-prefixed bindings
Fixes#12733
Unsure if there's a better way to prevent this kind of false positive but this is the one that made most sense to me.
In my experience, prefixing bindings with an underscore is the usual way to name variables that aren't used and that exist purely for executing drop code at the end of the scope.
-------
changelog: suppress [`readonly_write_lock`] for underscore-prefixed bindings
clippy::single_match(_else) may be machine applicable
```
changelog: [`single_match`]: make the lint machine-applicable
changelog: [`single_match_else`]: make the lint machine-applicable
```
---
The lint doesn't use placeholders. I've tried it on my codebases, and all instances of it applied without problems.
check if closure as method arg has read access in [`collection_is_never_read`]
fixes: #11783
---
changelog: fix [`collection_is_never_read`] misfires when use `retain` for iteration
configurably allow `useless_vec` in tests
This adds a `àllow-useless-vec-in-test` configuration which, when set to `true` will allow the `useless_vec` lint in `#[test]` functions and code within `#[cfg(test)]`. It also moves a `is_in_test` helper to `clippy_utils`.
---
changelog: configurably allow [`useless_vec`] in test code
This adds a `àllow-useless-vec-in-test` configuration which, when set
to `true` will allow the `useless_vec` lint in `#[test]` functions and
code within `#[cfg(test)]`. It also moves a `is_in_test` helper to
`clippy_utils`.
fix [`large_stack_arrays`] linting in `vec` macro
fixes: #12586
this PR also adds a wrapper function `matching_root_macro_call` to `clippy_utils::macros`, considering how often that same pattern appears in the codebase.
(I'm always very indecisive towards naming, so, if anyone have better idea of how that function should be named, feel free to suggest it)
---
changelog: fix [`large_stack_arrays`] linting in `vec` macro; add `matching_root_macro_call` to clippy_utils
[`non_canonical_partial_ord_impl`]: Fix emitting warnings which conflict with `needless_return`
fixes#12683
---
changelog: fix [`non_canonical_partial_ord_impl`] emitting warnings which conflict with `needless_return`
reduce `single_char_pattern` to only lint on ascii chars
This should mostly fix the `single_char_pattern` lint, because with a single byte, the optimizer will usually see through the char-to-string-expansion and single loop iteration. This fixes#11675 and #8111.
Update: As per the meeting on November 28th, 2023, we voted to also downgrade the lint to pedantic.
---
changelog: downgrade [`single_char_pattern`] to `pedantic`
Rework interior mutability detection
Replaces the existing interior mutability detection, the two main changes being
- It now follows references/pointers e.g. `struct S(&Cell)`
- `mutable_key_type` ignores pointers as it did before
- The `ignore_interior_mutability` config now applies to types containing the ignored type, e.g. `http::HeaderName`
Fixes https://github.com/rust-lang/rust-clippy/issues/7752
Fixes https://github.com/rust-lang/rust-clippy/issues/9776
Fixes https://github.com/rust-lang/rust-clippy/issues/9801
changelog: [`mutable_key_type`], [`declare_interior_mutable_const`]: now considers types that have references to interior mutable types as interior mutable
This commit introduces a check to ensure that the lint won't trigger when the initializer is
unreachable, such as:
```
thread_local! {
static STATE: Cell<usize> = panic!();
}
```
This is achieved by looking at the unpeeled initializer expression and ensuring that the parent
macro is not `panic!()`, `todo!()`, `unreachable!()`, `unimplemented!()`.
fixes#12637
changelog: [`threadlocal_initializer_can_be_made_const`] will no longer trigger on `unreachable` macros.
Emit the `needless_pass_by_ref_mut` lint on `self` arguments as well
Fixes https://github.com/rust-lang/rust-clippy/issues/12589.
Fixes https://github.com/rust-lang/rust-clippy/issues/9591.
The first commit fixes a bug I uncovered while working on this: sometimes, the mutable borrow "event" happens before the alias one, which makes some argument detected as not used mutably even if they are. The fix was simply to fill the map with the aliases afterwards.
The second commit removes the restriction to not run `self` argument for the `needless_pass_by_ref_mut` lint.
changelog: emit the `needless_pass_by_ref_mut` lint on `self` arguments as well
r? `@Manishearth`
Currently `SourceMap` is constructed slightly later than
`SessionGlobals`, and inserted. This commit changes things so they are
done at the same time.
Benefits:
- `SessionGlobals::source_map` changes from
`Lock<Option<Lrc<SourceMap>>>` to `Option<Lrc<SourceMap>>`. It's still
optional, but mutability isn't required because it's initialized at
construction.
- `set_source_map` is removed, simplifying `run_compiler`, which is
good because that's a critical function and it's nice to make it
simpler.
This requires moving things around a bit, so the necessary inputs are
available when `SessionGlobals` is created, in particular the `loader`
and `hash_kind`, which are no longer computed by `build_session`. These
inputs are captured by the new `SourceMapInputs` type, which is threaded
through various places.
Fix pretty HIR for anon consts in diagnostics
This removes the `NoAnn` printer which skips over nested bodies altogether, which is confusing, and requires users of `{ty|qpath|pat}_to_string` to pass in `&tcx` which now impleemnts `hir_pretty::PpAnn`.
There's one case where this "regresses" by actually printing out the body of the anon const -- we could suppress that, but I don't expect people to actually get anon consts like that unless they're fuzzing, tbh.
r? estebank
Cleanup: Rename `ModSep` to `PathSep`
`::` is usually referred to as the *path separator* (citation needed).
The existing name `ModSep` for *module separator* is a bit misleading since it in fact separates the segments of arbitrary path segments, not only ones resolving to modules. Let me just give a shout-out to associated items (`T::Assoc`, `<Ty as Trait>::function`) and enum variants (`Option::None`).
Motivation: Reduce friction for new contributors, prevent potential confusion.
cc `@petrochenkov`
r? nnethercote or compiler
fix: incorrect suggestions when `.then` and `.then_some` is used
fixes#11910
In the current implementation of `search_is_some`, if a `.is_none` call is followed by a `.then` or `.then_some` call, the generated `!` will incorrectly negate the values returned by the `then` and `.then_some` calls. To fix this, we need to add parentheses to the generated suggestions when appropriate.
changelog: [`search_is_some`]: add parenthesis to suggestions when appropriate
[`module_name_repetition`] Recognize common prepositions
Fixes#12544
changelog: [`module_name_repetition`]: don't report an item name if it consists only of a prefix from `allowed-prefixes` list and a module name (e.g. `AsFoo` in module `foo`). Prefixes allowed by default: [`to`, `from`, `into`, `as`, `try_into`, `try_from`]
Use `check_attributes` in doc lints
Ensures we catch all the places that doc comments could occur, found one that we were currently missing - docs on `extern` items
changelog: none
Fixes#12544.
- don't report an item name if it consists only of a prefix from `allowed-prefixes` list and a module name (e.g. `AsFoo` in module `foo`).
- configured by `allowed-prefixes` config entry
- prefixes allowed by default: [`to`, `from`, `into`, `as`, `try_into`, `try_from`]
- update docs
Correct parentheses for [`needless_borrow`] suggestion
This fixes#12268
Clippy no longer adds unnecessary parentheses in suggestions when the expression is part of a tuple.
---
changelog: Fix [`needless_borrow`] unnecessary parentheses in suggestion.
Implement minimal, internal-only pattern types in the type system
rebase of https://github.com/rust-lang/rust/pull/107606
You can create pattern types with `std::pat::pattern_type!(ty is pat)`. The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via `transmute`.
This PR's implementation differs from the MCP's text. Specifically
> This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types.
is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type `std::time::Nanoseconds` struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field.
Waiting on:
* [x] move all unrelated commits into their own PRs.
* [x] fix niche computation (see 2db07f94f44f078daffe5823680d07d4fded883f)
* [x] add lots more tests
* [x] T-types MCP https://github.com/rust-lang/types-team/issues/126 to finish
* [x] some commit cleanup
* [x] full self-review
* [x] remove 61bd325da19a918cc3e02bbbdce97281a389c648, it's not necessary anymore I think.
* [ ] ~~make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests)~~ we don't even do this for the new float primitives
* [x] get approval that [the scope expansion to trait impls](https://rust-lang.zulipchat.com/#narrow/stream/326866-t-types.2Fnominated/topic/Pattern.20types.20types-team.23126/near/427670099) is ok
r? `@BoxyUwU`
Stop exporting `TypeckRootCtxt` and `FnCtxt`.
While they have many convenient APIs, it is better to expose dedicated functions for them
noticed in #122213
Add consistency with phrases "meantime" and "mean time"
"mean time" is used in a few places while "meantime" is used everywhere else; this would make usage consistent throughout the codebase.
fix incorrect suggestion for `!(a as type >= b)`
fixes#12625
The expression `!(a as type >= b)` got simplified to `a as type < b`, but because of rust's parsing rules that `<` is interpreted as a start of generic arguments for `type`. This is fixed by recognizing this case and adding extra parens around the left-hand side of the comparison.
changelog: [`nonminimal_bool`]: fix incorrect suggestion for `!(a as type >= b)`
[`manual_unwrap_or_default`]: Check for Default trait implementation in initial condition when linting and use `IfLetOrMatch`
Fixes#12564
changelog: Fix [`manual_unwrap_or_default`] false positive when initial `match`/`if let` condition doesn't implement `Default` but the return type does.
Allow `cast` lints in macros
closes: #11738
Removed the `from_expansion` guard clause for cast lints, so that these warnings can be generated for internal macros.
changelog: allow `cast` lints in macros
Reword `arc_with_non_send_sync` note and help messages
Addresses https://github.com/rust-lang/rust-clippy/issues/12608#issuecomment-2029688054
Makes the note more concise and reframes the `Rc` suggestion around whether it crosses threads currently due to a manual `Send`/`Sync` impl or may do in the future
changelog: none
FIX(12334): manual_swap auto fix
Fixed: #12334
Initialization expressions are now generated as needed if the slice index is bound to a variable.
----
changelog: Fix [`manual_swap`]
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:
Move `iter_nth` to `style`, add machine applicable suggestion
There's no `O(n)` involved with `.iter().nth()` on the linted types since the iterator implementations provide `nth` and/or `advance_by` that operate in `O(1)`
For slice iterators the codegen is equivalent, `VecDeque`'s iterator seems to codegen differently but that doesn't seem significant enough to keep it as a perf lint
changelog: [`iter_nth`] Move to `style`
r? `@flip1995`
[`manual_retain`]: Fix duplicate diagnostics
Relates to: #12379
The first lint guard executed in `LateLintPass::check_expr` was testing if the parent was of type `ExprKind::Assign`. This meant the lint emitted on both sides of the assignment operator when `check_expr` is called on either `Expr`. The guard in the fix only lints once when the `Expr` is of kind `Assign`.
changelog: Fix duplicate lint diagnostic emission from [`manual_retain`]
Add new `duplicated_attributes` lint
It's a lint idea that `@llogiq` gave me while reviewing another PR.
There are some limitations, in particular for the "output". Initially I wanted to make it possible for directly lint against the whole attribute if its parts were all duplicated, but then I realized that the output would be chaotic if the duplicates were coming from different attributes, so I preferred to go to the simplest way and simply emit a warning for each entry. Not the best, but makes the implementation much easier.
Another limitation is that `cfg_attr` would be a bit more tricky to implement because we need to check if two `cfg` sets are exactly the same. I added a FIXME and will likely come back to it later.
And finally, I updated the `cargo dev update_lints` command because the generated `tests/ui/rename.rs` file was emitting the `duplicated_attributes` lint, so I allowed this lint inside it to prevent it from working.
changelog: Add new `duplicated_attributes` lint
[`single_match`]: Fix duplicate diagnostics
Relates to #12379
edit two test file
`tests/ui/single_match_else.rs`
`tests/ui/single_match.rs`
those two test file point to the same lint
---
changelog: [`single_match`] Fix duplicate diagnostics
[`mut_mut`]: Fix duplicate diags
Relates to #12379
The `mut_mut` lint produced two diagnostics for each `mut mut` pattern in `ty` inside `block`s because `MutVisitor::visit_ty` was called from `MutMut::check_ty` and `MutMut::check_block` independently. This PR fixes the issue.
---
changelog: [`mut_mut`]: Fix duplicate diagnostics
New lint `const_is_empty`
This lint detects calls to `.is_empty()` on an entity initialized from a string literal and flag them as suspicious. To avoid triggering on macros called from generated code, it checks that the `.is_empty()` receiver, the call itself and the initialization come from the same context.
Fixes#12307
changelog: [`const_is_empty`]: new lint
fix [`missing_docs_in_private_items`] on some proc macros
fixes: #12197
---
changelog: [`missing_docs_in_private_items`] support manually search for docs as fallback method
Add asm goto support to `asm!`
Tracking issue: #119364
This PR implements asm-goto support, using the syntax described in "future possibilities" section of [RFC2873](https://rust-lang.github.io/rfcs/2873-inline-asm.html#asm-goto).
Currently I have only implemented the `label` part, not the `fallthrough` part (i.e. fallthrough is implicit). This doesn't reduce the expressive though, since you can use label-break to get arbitrary control flow or simply set a value and rely on jump threading optimisation to get the desired control flow. I can add that later if deemed necessary.
r? ``@Amanieu``
cc ``@ojeda``
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
Remove double expr lint
Related to #12379.
Previously the code manually checked nested binop exprs in unary exprs, but those were caught anyway by `check_expr`. Removed that code path, the path is used in the tests.
---
changelog: [`nonminimal_bool`] Remove duplicate output on nested Binops in Unary exprs.
Add `assigning_clones` lint
This PR is a "revival" of https://github.com/rust-lang/rust-clippy/pull/10613 (with `@kpreid's` permission).
I tried to resolve most of the unresolved things from the mentioned PR:
1) The lint now checks properly if we indeed call the functions `std::clone::Clone::clone` or `std::borrow::ToOwned::to_owned`.
2) It now supports both method and function (UFCS) calls.
3) A heuristic has been added to decide if the lint should apply. It will only apply if the type on which the method is called has a custom implementation of `clone_from/clone_into`. Notably, it will not trigger for types that use `#[derive(Clone)]`.
4) `Deref` handling has been (hopefully) a bit improved, but I'm not sure if it's ideal yet.
I also added a bunch of additional tests.
There are a few things that could be improved, but shouldn't be blockers:
1) When the right-hand side is a function call, it is transformed into e.g. `::std::clone::Clone::clone(...)`. It would be nice to either auto-import the `Clone` trait or use the original path and modify it (e.g. `clone::Clone::clone` -> `clone::Clone::clone_from`). I don't know how to modify the `QPath` to do that though.
2) The lint currently does not trigger when the left-hand side is a local variable without an initializer. This is overly conservative, since it could trigger when the variable has no initializer, but it has been already initialized at the moment of the function call, e.g.
```rust
let mut a;
...
a = Foo;
...
a = b.clone(); // Here the lint should trigger, but currently doesn't
```
These cases probably won't be super common, but it would be nice to make the lint more precise. I'm not sure how to do that though, I'd need access to some dataflow analytics or something like that.
changelog: new lint [`assigning_clones`]
[`misrefactored_assign_op`]: Fix duplicate diagnostics
Relate to #12379
The following diagnostics appear twice
```
--> tests/ui/assign_ops2.rs:26:5
|
LL | a *= a * a;
| ^^^^^^^^^^
|
help: did you mean `a = a * a` or `a = a * a * a`? Consider replacing it with
```
because `a` (lhs) appears in both left operand and right operand in the right hand side.
This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.
changelog: [`misrefactored_assign_op`]: Fix duplicate diagnostics
Don't emit "missing backticks" lint if the element is wrapped in `<code>` HTML tags
Fixes#9473.
changelog: Don't emit "missing backticks" lint if the element is wrapped in `<code>` HTML tags
Existing names for values of this type are `sess`, `parse_sess`,
`parse_session`, and `ps`. `sess` is particularly annoying because
that's also used for `Session` values, which are often co-located, and
it can be difficult to know which type a value named `sess` refers to.
(That annoyance is the main motivation for this change.) `psess` is nice
and short, which is good for a name used this much.
The commit also renames some `parse_sess_created` values as
`psess_created`.
Add missing header for `manual_is_variant_and`
Noticed this while generating our lint completions failed in rust-analyzer (separate PR from https://github.com/rust-lang/rust-clippy/pull/12415 as I made these via the github interface quickly)
changelog: none
[`identity_op`]: Fix duplicate diagnostics
Relates to #12379
In the `identity_op` lint, the following diagnostic was emitted two times
```
--> tests/ui/identity_op.rs:156:5
|
LL | 1 * 1;
| ^^^^^ help: consider reducing it to: `1`
|
```
because both of the left operand and the right operand are the identity element of the multiplication.
This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.
---
changelog: [`identity_op`]: Fix duplicate diagnostics
Check for try blocks in `question_mark` more consistently
Fixes#12337
I split this PR up into two commits since this moves a method out of an `impl`, which makes for a pretty bad diff (the `&self` parameter is now unused, and there isn't a reason for that function to be part of the `impl` now).
The first commit is the actual relevant change and the 2nd commit just moves stuff (github's "hide whitespace" makes the diff easier to look at)
------------
Now for the actual issue:
`?` within `try {}` blocks desugars to a `break` to the block, rather than a `return`, so that changes behavior in those cases.
The lint has multiple patterns to look for and in *some* of them it already does correctly check whether we're in a try block, but this isn't done for all of its patterns.
We could add another `self.inside_try_block()` check to the function that looks for `let-else-return`, but I chose to actually just move those checks out and instead have them in `LintPass::check_{stmt,expr}`. This has the advantage that we can't (easily) accidentally forget to add that check in new patterns that might be added in the future.
(There's also a bit of a subtle interaction between two lints, where `question_mark`'s LintPass calls into `manual_let_else`, so I added a check to make sure we don't avoid linting for something that doesn't have anything to do with `?`)
changelog: [`question_mark`]: avoid linting on try blocks in more cases
fix [`derive_partial_eq_without_eq`] FP on trait projection
fixes: #9413#9319
---
changelog: fix [`derive_partial_eq_without_eq`] FP on trait projection
Well, this is awkward, it works but I don't understand why, why `clippy_utils::ty::implements_trait` couldn't detects the existance of `Eq` trait, even thought it's obviously present in the derive attribute.
Pointers cannot be converted to integers at compile time
Fix#12402
changelog: [`transmutes_expressible_as_ptr_casts`]: do not suggest invalid const casts
Dedup std_instead_of_core by using first segment span for uniqueness
Relates to #12379.
Instead of checking that the paths have an identical span, it checks that the relevant `std` part of the path segment's span is identical. Added a multiline test, because my first implementation was worse and failed that, then I realized that you could grab the span off the first_segment `Ident`.
I did find another bug that isn't addressed by this, and that exists on master as well.
The path:
```Rust
use std::{io::Write, fmt::Display};
```
Will get fixed into:
```Rust
use core::{io::Write, fmt::Display};
```
Which doesn't compile since `io::Write` isn't in `core`, if any of those paths are present in `core` it'll do the replace and cause a miscompilation. Do you think I should file a separate bug for that? Since `rustfmt` default splits those up it isn't that big of a deal.
Rustfmt:
```Rust
// Pre
use std::{io::Write, fmt::Display};
// Post
use std::fmt::Display;
use std::io::Write;
```
---
changelog: [`std_instead_of_core`]: Fix duplicated output on multiple imports
fix: `manual_memcpy` wrong indexing for multi dimensional arrays
fixes: #9334
This PR fixes an invalid suggestion for multi-dimensional arrays.
For example,
```rust
let src = vec![vec![0; 5]; 5];
let mut dst = vec![0; 5];
for i in 0..5 {
dst[i] = src[i][i];
}
```
For the above code, Clippy suggests `dst.copy_from_slice(&src[i]);`, but it is not compilable because `i` is only used to loop the array.
I adjusted it so that Clippy `manual_memcpy` works properly for multi-dimensional arrays.
changelog: [`manual_memcpy`]: Fixes invalid indexing suggestions for multi-dimensional arrays
Add stubs in IR and ABI for `f16` and `f128`
This is the very first step toward the changes in https://github.com/rust-lang/rust/pull/114607 and the [`f16` and `f128` RFC](https://rust-lang.github.io/rfcs/3453-f16-and-f128.html). It adds the types to `rustc_type_ir::FloatTy` and `rustc_abi::Primitive`, and just propagates those out as `unimplemented!` stubs where necessary.
These types do not parse yet so there is no feature gate, and it should be okay to use `unimplemented!`.
The next steps will probably be AST support with parsing and the feature gate.
r? `@compiler-errors`
cc `@Nilstrieb` suggested breaking the PR up in https://github.com/rust-lang/rust/pull/120645#issuecomment-1925900572
`os_local` impl of `thread_local` — regardless of whether it is const and
unlike other implementations — includes an `fn __init(): EXPR`.
Existing implementation of the lint checked for the presence of said
function and whether the expr can be made const. Because for `os_local`
we always have an `__init()`, it triggers for const implementations.
The solution is to check whether the `__init()` function is already const.
If it is `const`, there is nothing to do. Otherwise, we verify that we can
make it const.
Co-authored-by: Alejandra González <blyxyas@gmail.com>
Add new `mixed_attributes_style` lint
Add a new lint to detect cases where both inner and outer attributes are used on a same item.
r? `@llogiq`
----
changelog: Add new [`mixed_attributes_style`] lint
The following code used to trigger the lint:
```rs
macro_rules! make_closure {
() => {
(|| {})
};
}
make_closure!()();
```
The lint would suggest to replace `make_closure!()()` with
`make_closure!()`, which changes the code and removes the call to the
closure from the macro. This commit fixes that.
Fixes#12358
Fix `nonminimal_bool` lint regression
Fixes#12371.
Fixes#12369.
cc `@RalfJung`
The problem was an invalid condition. Shame on me...
changelog: Fix `nonminimal_bool` lint regression
[`map_entry`]: Check insert expression for map use
The lint makes sure that the map is not used (borrowed) before the call to `insert`. Since the lint creates a mutable borrow on the map with the `Entry`, it wouldn't be possible to replace such code with `Entry`. However, expressions up to the `insert` call are checked, but not expressions for the arguments of the `insert` call itself. This commit fixes that.
Fixes#11935
----
changelog: [`map_entry`]: Fix false positive when borrowing the map in the `insert` call
If the whole cast expression is a unary expression (`(*x as T)`) or an
addressof expression (`(&x as T)`), then not surrounding the suggestion
into a block risks us changing the precedence of operators if the cast
expression is followed by an operation with higher precedence than the
unary operator (`(*x as T).foo()` would become `*x.foo()`, which changes
what the `*` applies on).
The same is true if the expression encompassing the cast expression is a
unary expression or an addressof expression.
The lint supports the latter case, but missed the former one. This PR
fixes that.
Fixes#11968
The lint makes sure that the map is not used (borrowed) before the call
to `insert`. Since the lint creates a mutable borrow on the map with the
`Entry`, it wouldn't be possible to replace such code with `Entry`.
However, expressions up to the `insert` call are checked, but not
expressions for the arguments of the `insert` call itself. This commit
fixes that.
Fixes#11935
Fix sign-handling bugs and false negatives in `cast_sign_loss`
**Note: anyone should feel free to move this PR forward, I might not see notifications from reviewers.**
changelog: [`cast_sign_loss`]: Fix sign-handling bugs and false negatives
This PR fixes some arithmetic bugs and false negatives in PR #11883 (and maybe earlier PRs).
Cc `@J-ZhengLi`
I haven't updated the tests yet. I was hoping for some initial feedback before adding tests to cover the cases listed below.
Here are the issues I've attempted to fix:
#### `abs()` can return a negative value in release builds
Example:
```rust
i32::MIN.abs()
```
https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=022d200f9ef6ee72f629c0c9c1af11b8
Docs: https://doc.rust-lang.org/std/primitive.i32.html#method.abs
Other overflows that produce negative values could cause false negatives (and underflows could produce false positives), but they're harder to detect.
#### Values with uncertain signs can be positive or negative
Any number of values with uncertain signs cause the whole expression to have an uncertain sign, because an uncertain sign can be positive or negative.
Example (from UI tests):
```rust
fn main() {
foo(a: i32, b: i32, c: i32) -> u32 {
(a * b * c * c) as u32
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
}
println!("{}", foo(1, -1, 1));
}
```
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=165d2e2676ee8343b1b9fe60db32aadd
#### Handle `expect()` the same way as `unwrap()`
Since we're ignoring `unwrap()` we might as well do the same with `expect()`.
This doesn't seem to have tests but I'm happy to add some like `Some(existing_test).unwrap() as u32`.
#### A negative base to an odd exponent is guaranteed to be negative
An integer `pow()`'s sign is only uncertain when its operants are uncertain. (Ignoring overflow.)
Example:
```rust
((-2_i32).pow(3) * -2) as u32
```
This offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. (Rather than just an odd number of uncertain signs.)
#### Both sides of a multiply or divide should be peeled recursively
I'm not sure why the lhs was peeled recursively, and the rhs was left intact. But the sign of any sequence of multiplies and divides is determined by the signs of its operands. (Ignoring overflow.)
I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.
But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, these should all lint:
```rust
fn peel_all(x: i32) {
(-p(x) * -p(x) * -p(x)) as u32;
((-p(x) * -p(x)) * -p(x)) as u32;
(-p(x) * (-p(x) * -p(x))) as u32;
}
```
#### The right hand side of a Rem doesn't change the sign
Unlike Mul and Div,
> Given remainder = dividend % divisor, the remainder will have the same sign as the dividend.
https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators
I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.
But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, only the first six expressions should lint.
The expressions that start with a constant should lint (or not lint) regardless of whether the lint supports `p()` or unary negation, because only the dividend's sign matters.
Example:
```rust
fn rem_lhs(x: i32) {
(-p(x) % -1) as u32;
(-p(x) % 1) as u32;
(-1 % -p(x)) as u32;
(-1 % p(x)) as u32;
(-1 % -x) as u32;
(-1 % x) as u32;
// These shouldn't lint:
(p(x) % -1) as u32;
(p(x) % 1) as u32;
(1 % -p(x)) as u32;
(1 % p(x)) as u32;
(1 % -x) as u32;
(1 % x) as u32;
}
```
#### There's no need to bail on other expressions
When peeling, any other operators or expressions can be left intact and sent to the constant evaluator.
If these expressions can be evaluated, this offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. If not, they end up marked as having uncertain sign.
[`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls
This lint now also realizes that a comparison like `s == "foo"` and calls such as `s.ends_with("foo")` will fail if `s` was initialized by a call to `Stdin::read_line` (because of the trailing newline).
changelog: [`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls
r? `@giraffate` assigning you because you reviewed #10970 that added this lint, so this is kinda a followup PR ^^
fix suggestion error in [`useless_vec`]
fixes: #12101
---
changelog: fix suggestion error in [`useless_vec`]
r+ `@matthiaskrgr` since they opened the issue?
Empty docs
Fixes https://github.com/rust-lang/rust-clippy/issues/9931
changelog: [`empty_doc`]: Detects documentation that is empty.
changelog: Doc comment lints now trigger for struct field and enum variant documentation
When encountering code such as:
```
Box::new(outer::Inner::default())
```
clippy would suggest replacing with `Box::<Inner>::default()`, dropping
the `outer::` segment. This behavior is incorrect and that commit fixes
it.
What it does is it checks the contents of the `Box::new` and, if it is
of the form `A::B::default`, does a text replacement, inserting `A::B`
in the `Box`'s quickfix generic list.
If the source does not match that pattern (including `Vec::from(..)`
or other `T::new()` calls), we then fallback to the original code.
Fixes#11927
Look for `implied_bounds_in_impls` in more positions
With this, we lint `impl Trait` implied bounds in more positions:
- Type alias impl trait
- Associated type position impl trait
- Argument position impl trait
- these are not opaque types, but instead are desugared to `where` clauses, so we need extra logic for finding them (`check_generics`), however the rest of the logic is the same
Before this, we'd only lint RPIT `impl Trait`s.
"Hide whitespaces" and reviewing commits individually might make this easier
changelog: [`implied_bounds_in_impls`]: start linting implied bounds in APIT, ATPIT, TAIT
Merge `single_call_fn` post-crate visitor into lint pass
The `single_call_fn` lint worked by first collecting a list of function definitions in the lint pass, then populating the list of uses for each function in a second visitor after the crate is checked.
Doing another pass through the crate shouldn't be needed, and we should be able to do it in the same lint pass, by looking for path references to functions only and then processing them post-crate.
Other changes:
- `FxHashMap` -> `FxIndexMap` so that we emit warnings in a consistent order, as we see them (making the diff a bit confusing to look at, because warnings were moved around)
- no longer storing a `Vec<Span>` per function: an enum representing "seen once" or "seen more than once" should be enough (only the first element is used later)
- "used here" help is now a note
I also noticed that it lints on trait methods with a default implementation, but not on regular trait methods without a body (because that's what `check_fn` does). I'm not sure if that's useful though, maybe we shouldn't lint trait methods at all? It's not like you can avoid it sometimes (but then again it's a restriction lint). Either way, I left the behavior where it was before so that there are no functional changes made in this PR and it's purely a refactor. I can change it though
changelog: none
FIX(12243): redundant_guards
Fixed#12243
changelog: Fix[`redundant_guards`]
I have made a correction so that no warning does appear when y.is_empty() is used within a constant function as follows.
```rust
pub const fn const_fn(x: &str) {
match x {
// Shouldn't lint.
y if y.is_empty() => {},
_ => {},
}
}
```
Add new `unnecessary_get_then_check` lint
No issue linked to this as far as I can see. It's a lint I discovered that could be added when I worked on another lint.
r? `@llogiq`
changelog: Add new `unnecessary_get_then_check` lint
A warning is now suppressed when "<str_va> if <str_var>.is_empty" is used in a constant function.
FIX: instead of clippy_util::in_const
FIX: Merged `redundant_guards_const_fn.rs` into `redundant_guards.rs`.
Extend `unnecessary_to_owned` to handle `Borrow` trait in map types
Fixes https://github.com/rust-lang/rust-clippy/issues/8088.
Alternative to #12315.
r? `@y21`
changelog: Extend `unnecessary_to_owned` to handle `Borrow` trait in map types
----
UPDATE: add async block into test.
FIX: no_effect
Fixed asynchronous function parameter names with underscores so that warnings are not displayed when underscores are added to parameter names
ADD: test case
There are lots of functions that modify a diagnostic. This can be via a
`&mut Diagnostic` or a `&mut DiagnosticBuilder`, because the latter type
wraps the former and impls `DerefMut`.
This commit converts all the `&mut Diagnostic` occurrences to `&mut
DiagnosticBuilder`. This is a step towards greatly simplifying
`Diagnostic`. Some of the relevant function are made generic, because
they deal with both errors and warnings. No function bodies are changed,
because all the modifier methods are available on both `Diagnostic` and
`DiagnosticBuilder`.
This mostly works well, and eliminates a couple of delayed bugs.
One annoying thing is that we should really also add an
`ErrorGuaranteed` to `proc_macro::bridge::LitKind::Err`. But that's
difficult because `proc_macro` doesn't have access to `ErrorGuaranteed`,
so we have to fake it.
Ignore imported items in `min_ident_chars`
Suppress the `min_ident_chars` warning for items whose name we cannot control. Do not warn for `use a::b`, but warn for `use a::b as c`, since `c` is a local identifier.
Fixes#12232
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`min_ident_chars`]: Do not warn on non-local identifiers
[`incompatible_msrv`]: allow expressions that come from desugaring
Fixes#12273
changelog: [`incompatible_msrv`]: don't lint on the `IntoFuture::into_future` call desugared by `.await`
Suppress the `min_ident_chars` warning for items whose name we cannot
control. Do not warn for `use a::b`, but warn for `use a::b as c`, since
`c` is a local identifier.
Fixes#12232
[`implied_bounds_in_impls`]: avoid linting on overlapping associated tys
Fixes#11880
Before this change, we were simply ignoring associated types (except for suggestion purposes), because of an incorrect assumption (see the comment that I also removed).
For something like
```rs
trait X { type T; }
trait Y: X { type T; }
// Can't constrain `X::T` through `Y`
fn f() -> impl X<T = i32> + Y<T = u32> { ... }
```
We now avoid linting if the implied bound (`X<T = i32>`) "names" associated types that also exists in the implying trait (`trait Y`). Here that would be the case.
But if we only wrote `impl X + Y<T = u32>` then that's ok because `X::T` was never constrained in the first place.
I haven't really thought about how this interacts with GATs, but I think it's fine. Fine as in, it might create false negatives, but hopefully no false positives.
(The diff is slightly annoying because of formatting things. Really the only thing that changed in the if chain is extracting the `implied_by_def_id` which is needed for getting associated types from the trait, and of course actually checking for overlap)
cc `@Jarcho` ? idk if you want to review this or not. I assume you looked into this code a bit to find this bug.
changelog: [`implied_bounds_in_impls`]: avoid linting when associated type from supertrait can't be constrained through the implying trait bound
[`mem_replace_with_default`] No longer triggers on unused expression
changelog:[`mem_replace_with_default`]: No longer triggers on unused expression
Change [`mem_replace_with_default`] to not trigger on unused expression because the lint from `#[must_use]` handle this case better.
fixes: #5586
Minor refactor format-args
* Move all linting logic into a single format implementations struct
This should help with the future format-args improvements.
**NOTE TO REVIEWERS**: use "hide whitespace" in the github diff -- most of the code has shifted, but relatively low number of lines actually modified.
Followig up from #12274
r? `@xFrednet`
---
changelog: none
Fix async closures in CTFE
First commit renames `is_coroutine_or_closure` into `is_closure_like`, because `is_coroutine_or_closure_or_coroutine_closure` seems confusing and long.
Second commit fixes some forgotten cases where we want to handle `TyKind::CoroutineClosure` the same as closures and coroutines.
The test exercises the change to `ValidityVisitor::aggregate_field_path_elem` which is the source of #120946, but not the change to `UsedParamsNeedSubstVisitor`, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against `TooGeneric` anywhere?).
Fixes#120946
r? oli-obk cc ``@RalfJung``
fix: ICE when array index exceeds usize
fixes#12253
This PR fixes ICE in `indexing_slicing` as it panics when the index of the array exceeds `usize`.
changelog: none
Don't allow derive macros to silence `disallowed_macros`
fixes#12254
The implementation is a bit of a hack, but "works". A derive expanding to another derive won't work properly, but we shouldn't be linting those anyways.
changelog: `disallowed_macros`: Don't allow derive macros to silence their own expansion
Assert that params with the same *index* have the same *name*
Found this bug when trying to build libcore with the new solver, since it will canonicalize two params with the same index into *different* placeholders if those params differ by name.
Minor refactor format-impls
Move all linting logic into a single format implementations struct
This should help with the future format-args improvements.
TODO: do the same with format_args.rs, perhaps in the same PR
**NOTE TO REVIEWERS**: use "hide whitespace" in the github diff -- most of the code has shifted, but relatively low number of lines actually modified.
changelog: none
Refactor `implied_bounds_in_impls` lint
Some refactors in `implied_bounds_in_impls` that I wanted to make while working on something else in that file, but I found them "large" enough that I didn't want them in the same PR and instead wanted them reviewed separately (since itd just be distracting).
This just splits up the two phases of "collect all the supertraits from each of the `impl Trait` bounds" and "find those `impl Trait` bounds that are mentioned in one of the previously-collected supertraits" into separate functions. Before, this was all in a single function.
Reviewing it commit by commit might make it easier. I can squash it down later.
changelog: none
stop linting [`blocks_in_conditions`] on `match` with weird attr macro case
should fixes: #12016
---
changelog: [`blocks_in_conditions`] - fix FP on `match` with weird attr macro
This might not be the best solution, as the root cause (i think?) is the `span` of block was incorrectly given by the compiler?
I'm open to better solutions
* Move all linting logic into a single format implementations struct
This should help with the future format-args improvements.
TODO: do the same with format_args.rs, perhaps in the same PR
A lot of cases of the "noise" cases of `similar_names` come from two
idents with a different first letter, which is easy enough to
differentiate visually but causes this lint to be raised.
Do not raise the lint in these cases, as long as the first character
does not have a lookalike.
Link: https://github.com/rust-lang/rust-clippy/issues/10926
Fix issue #12034: add autofixes for unnecessary_fallible_conversions
fixes#12034
Currently, the `unnecessary_fallible_conversions` lint was capable of autofixing expressions like `0i32.try_into().unwrap()`. However, it couldn't autofix expressions in the form of `i64::try_from(0i32).unwrap()` or `<i64 as TryFrom<i32>>::try_from(0).unwrap()`.
This pull request extends the functionality to correctly autofix these latter forms as well.
changelog: [`unnecessary_fallible_conversions`]: Add autofixes for more forms
Invert diagnostic lints.
That is, change `diagnostic_outside_of_impl` and `untranslatable_diagnostic` from `allow` to `deny`, because more than half of the compiler has been converted to use translated diagnostics.
This commit removes more `deny` attributes than it adds `allow` attributes, which proves that this change is warranted.
r? ````@davidtwco````
Rollup of 9 pull requests
Successful merges:
- #119592 (resolve: Unload speculatively resolved crates before freezing cstore)
- #120103 (Make it so that async-fn-in-trait is compatible with a concrete future in implementation)
- #120206 (hir: Make sure all `HirId`s have corresponding HIR `Node`s)
- #120214 (match lowering: consistently lower bindings deepest-first)
- #120688 (GVN: also turn moves into copies with projections)
- #120702 (docs: also check the inline stmt during redundant link check)
- #120727 (exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered")
- #120734 (Add `SubdiagnosticMessageOp` as a trait alias.)
- #120739 (improve pretty printing for associated items in trait objects)
r? `@ghost`
`@rustbot` modify labels: rollup
[`unconditional_recursion`]: compare by `Ty`s instead of `DefId`s
Fixes#12154Fixes#12181 (this was later edited in, so the rest of the description refers to the first linked issue)
Before this change, the lint would work with `DefId`s and use those to compare types. This PR changes it to compare types directly. It fixes the linked issue, but also other false positives I found in a lintcheck run. For example, one of the issues is that some types don't have `DefId`s (primitives, references, etc., leading to possible FNs), and the helper function used to extract a `DefId` didn't handle type parameters.
Another issue was that the lint would use `.peel_refs()` in a few places where that could lead to false positives (one such FP was in the `http` crate). See the doc comment on one of the added functions and also the test case for what I mean.
The code in the linked issue was linted because the receiver type is `T` (a `ty::Param`), which was not handled in `get_ty_def_id` and returned `None`, so this wouldn't actually *get* to comparing `self_arg != ty_id` here, and skip the early-return:
70573af31e/clippy_lints/src/unconditional_recursion.rs (L171-L178)
This alone could be fixed by doing something like `&& get_ty_def_id(ty).map_or(true, |ty_id)| self_arg != ty_id)`, but we don't really need to work with `DefId`s in the first place, I don't think.
changelog: [`unconditional_recursion`]: avoid linting when the other comparison type is a type parameter
Fix false positive in `redundant_type_annotations` lint
This PR changes the `redundant_type_annotations` lint to allow slice type annotations (i.e., `&[u8]`) for byte string literals. It will still consider _array_ type annotations (i.e., `&[u8; 4]`) as redundant. The reasoning behind this is that the type of byte string literals is by default a reference to an array, but, by using a type annotation, you can force it to be a slice. For example:
```rust
let a: &[u8; 4] = b"test";
let b: &[u8] = b"test";
```
Now, the type annotation for `a` will still be linted (as it is still redundant), but the type annotation for `b` will not.
Fixes#12212.
changelog: [`redundant_type_annotations`]: Fix false positive with byte string literals
That is, change `diagnostic_outside_of_impl` and
`untranslatable_diagnostic` from `allow` to `deny`, because more than
half of the compiler has be converted to use translated diagnostics.
This commit removes more `deny` attributes than it adds `allow`
attributes, which proves that this change is warranted.
[`redundant_locals`]: take by-value closure captures into account
Fixes#12225
The same problem in the linked issue can happen to regular closures too, and conveniently async blocks are closures in the HIR so fixing closures will fix async blocks as well.
changelog: [`redundant_locals`]: avoid linting when redefined variable is captured by-value
Add new lint: `ref_as_ptr`
Fixes#10130
Added new lint `ref_as_ptr` that checks for conversions from references to pointers and suggests using `std::ptr::from_{ref, mut}` instead.
The name is different than suggested in the issue (`as_ptr_cast`) since there were some other lints with similar names (`ptr_as_ptr`, `borrow_as_ptr`) and I wanted to follow the convention.
Note that this lint conflicts with the `borrow_as_ptr` lint in the sense that it recommends changing `&foo as *const _` to `std::ptr::from_ref(&foo)` instead of `std::ptr::addr_of!(foo)`. Personally, I think the former is more readable and, in contrast to `addr_of` macro, can be also applied to temporaries (cf. #9884).
---
changelog: New lint: [`ref_as_ptr`]
[#12087](https://github.com/rust-lang/rust-clippy/pull/12087)
add configuration for [`wildcard_imports`] to ignore certain imports
fixes: #11428
changelog: add configuration `ignored-wildcard-imports` for lint [`wildcard_imports`]
Fixed FP in `unused_io_amount` for Ok(lit), unrachable! and unwrap de…
…sugar
Fixes fp caused by linting on Ok(_) for all cases outside binding.
We introduce the following rules for match exprs.
- `panic!` and `unreachable!` are treated as consumed.
- `Ok( )` patterns outside `DotDot` and `Wild` are treated as consuming.
changelog: FP [`unused_io_amount`] when matching Ok(literal) or unreachable
fixes#12208
r? `@blyxyas`
We introduce the following rules for match exprs.
- `panic!` and `unreachable!` are treated as consumption.
- guard expressions in any arm imply consumption.
For match exprs:
- Lint only if exacrtly 2 non-consuming arms exist
- Lint only if one arm is an `Ok(_)` and the other is `Err(_)`
Added additional requirement that for a block return expression
that is a match, the source must be `Normal`.
changelog: FP [`unused_io_amount`] when matching Ok(literal)
Remove various `has_errors` or `err_count` uses
follow up to https://github.com/rust-lang/rust/pull/119895
r? `@nnethercote` since you recently did something similar.
There are so many more of these, but I wanted to get a PR out instead of growing the commit list indefinitely. The commits all work on their own and can be reviewed commit by commit.
The query accept arbitrary DefIds, not just owner DefIds.
The return can be an `Option` because if there are no nodes, then it doesn't matter whether it's due to NonOwner or Phantom.
Also rename the query to `opt_hir_owner_nodes`.
Add regression ui test for #2371Fixes#2371.
#2371 seems to already be handled correctly in the lint. This PR adds a ui regression test so we can close it.
r? `@blyxyas`
changelog: Add regression ui test for #2371
[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules
Fixes#10854.
Currently, `redundant_closure_for_method_calls` suggest incorrect paths when a method defined on a struct within inline mod is referenced (see the description in the aforementioned issue for an example; also see [this playground link](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=f7d3c5b2663c9bd3ab7abdb0bd38ee43) for the current-version output for the test cases added in this PR). It will now try to construct a relative path path to the module and suggest it instead.
changelog: [`redundant_closure_for_method_calls`] Fix incorrect path suggestions for types within local modules
FP: `needless_return_with_question_mark` with implicit Error Conversion
Return with a question mark was triggered in situations where the `?` desuraging was performing error conversion via `Into`/`From`.
The desugared `?` produces a match over an expression with type `std::ops::ControlFlow<B,C>` with `B:Result<Infallible, E:Error>` and `C:Result<_, E':Error>`, and the arms perform the conversion. The patch adds another check in the lint that checks that `E == E'`. If `E == E'`, then the `?` is indeed unnecessary.
changelog: False Positive: [`needless_return_with_question_mark`] when implicit Error Conversion occurs.
fixes: #11982
fix: incorrect suggestions generated by `manual_retain` lint
fixes#10393, fixes#11457, fixes#12081#10393: In the current implementation of `manual_retain`, if the argument to the closure is matched using tuple, they are all treated as the result of a call to `map.into_iter().filter(<f>)`. However, such tuple pattern matching can also occur in many different containers that stores tuples internally. The correct approach is to apply different lint policies depending on whether the receiver of `into_iter` is a map or not.
#11457 and #12081: In the current implementation of `manual_retain`, if the argument to the closure is `Binding`, the closure will be used directly in the `retain` method, which will result in incorrect suggestion because the first argument to the `retain` closure may be of a different type. In addition, if the argument to the closure is `Ref + Binding`, the lint will simply remove the `Ref` part and use the `Binding` part as the argument to the new closure, which will lead to bad suggestion for the same reason. The correct approach is to detect each of these cases and apply lint suggestions conservatively.
changelog: [`manual_retain`] refactor and add check for various patterns
Fix/Issue11932: assert* in multi-condition after unrolling will cause lint `nonminimal_bool` emit warning
fixes [Issue#11932](https://github.com/rust-lang/rust-clippy/issues/11932)
After `assert`, `assert_eq`, `assert_ne`, etc, assert family marcos unrolling in multi-condition expressions, lint `nonminimal_bool` will recognize whole expression as a entirety, analyze each simple condition expr of them, and check whether can simplify them.
But `assert` itself is a entirety to programmers, we don't need to lint on `assert`. This commit add check whether lint snippet contains `assert` when try to warning to an expression.
changelog: [`nonminimal_bool`] add check for condition expression
[`never_loop`]: recognize desugared `try` blocks
Fixes#12205
The old code assumed that only blocks with an explicit label can be jumped to (using `break`). This is mostly correct except for `try` desugaring, where the `?` operator is rewritten to a `break` to that block, even without a label on the block. `Block::targeted_by_break` is a little more accurate than just checking if a block has a label in that regard, so we should just use that instead
changelog: [`never_loop`]: avoid linting when `?` is used inside of a try block
Fixed FP in `redundant_closure_call` when closures are passed to macros
There are cases where the closure call is needed in some macros, this in particular occurs when the closure has parameters. To handle this case, we allow the lint when there are no parameters in the closure, or the closure is outside a macro invocation.
fixes: #11274#1553
changelog: FP: [`redundant_closure_call`] when closures with parameters are passed in macros.
Warn if an item coming from more recent version than MSRV is used
Part of https://github.com/rust-lang/rust-clippy/issues/6324.
~~Currently, the lint is not working for the simple reason that the `stable` attribute is not kept in dependencies. I'll send a PR to rustc to see if they'd be okay with keeping it.~~
EDIT: There was actually a `lookup_stability` function providing this information, so all good now!
cc `@epage`
changelog: create new [`incompatible_msrv`] lint
Consolidating rustc Dependencies
changelog: none
For dependencies in rustc where there are multiple versions used, this moves the older dependency to the newer dependency. These are the updates to clippy as mentioned here: https://github.com/rust-lang/rust/pull/120177
[`multiple_crate_versions`]: add a configuration option for allowed duplicate crates
Closes#12176
changelog: [`multiple_crate_versions`]: add a configuration option for allowed duplicate crates
respect `#[allow]` attributes in `single_call_fn` lint
Fixes#12182
If we delay linting to `check_crate_post`, we need to use `span_lint_hir_and_then`, since otherwise it would only respect those lint level attributes at the crate root.
<sub>... maybe we can have an internal lint for this somehow?</sub>
changelog: respect `#[allow]` attributes in `single_call_fn` lint
Don't emit `derive_partial_eq_without_eq` lint if the type has the `non_exhaustive` attribute
Part of https://github.com/rust-lang/rust-clippy/issues/9063.
If a type has a field/variant with the `#[non_exhaustive]` attribute or the type itself has it, then do no emit the `derive_partial_eq_without_eq` lint.
changelog: Don't emit `derive_partial_eq_without_eq` lint if the type has the `non_exhaustive` attribute
Pack u128 in the compiler to mitigate new alignment
This is based on #116672, adding a new `#[repr(packed(8))]` wrapper on `u128` to avoid changing any of the compiler's size assertions. This is needed in two places:
* `SwitchTargets`, otherwise its `SmallVec<[u128; 1]>` gets padded up to 32 bytes.
* `LitKind::Int`, so that entire `enum` can stay 24 bytes.
* This change definitely has far-reaching effects though, since it's public.
`unused_io_amount` captures `Ok(_)`s
Partial rewrite of `unused_io_amount` to lint over `Ok(_)` and `Ok(..)`.
Moved the check to `check_block` to simplify context checking for expressions and allow us to check only some expressions.
For match (expr, arms) we emit a lint for io ops used on `expr` when an arm is `Ok(_)|Ok(..)`. Also considers the cases when there are guards in the arms and `if let Ok(_) = ...` cases.
For `Ok(_)` and `Ok(..)` it emits a note indicating where the value is ignored.
changelog: False Negatives [`unused_io_amount`]: Extended `unused_io_amount` to catch `Ok(_)`s in `If let` and match exprs.
Closes#11713
r? `@giraffate`
Partial rewrite of `unused_io_account` to lint over Ok(_).
Moved the check to `check_block` to simplify context checking for
expressions and allow us to check only some expressions.
For match (expr, arms) we emit a lint for io ops used on `expr` when an
arm is `Ok(_)`. Also considers the cases when there are guards in the
arms. It also captures `if let Ok(_) = ...` cases.
For `Ok(_)` it emits a note indicating where the value is ignored.
changelog: False Negatives [`unused_io_amount`]: Extended
`unused_io_amount` to catch `Ok(_)`s in `If let` and match exprs.
Don't forget that the lifetime on hir types is `'tcx`
This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for https://github.com/rust-lang/rust/pull/107606 (now #120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
no_effect_underscore_binding: _ prefixed variables can be used
Prefixing a variable with a `_` does not mean that it will not be used. If such a variable is used later, do not warn about the fact that its initialization does not have a side effect as this is fine.
changelog: [`no_effect_underscore_binding`]: warn only if variable is unused
Fix#12166
Add . to end of lint lists in configuration + Fix typo in pub_underscore_fields_behavior
Fixes https://github.com/rust-lang/rust-clippy/pull/10283#issuecomment-1890600381
In the "/// Lint: " list on each configuration option, you have to end with a dot. If the lint list doesn't have a dot, the configuration won't have documentation.
This PR adds those missing dots in some of the configuration, thus also adding their documentation.
changelog: Fix bug where a lot of config documentation wasn't showing.
changelog: Fix typo in `pub_underscore_fields_behavior` (`PublicallyExported` -> `PubliclyExported`)
Find function path references early in the same lint pass
This removes a visitor that existed to collect paths to functions in a context where the exact signature is required in order to cancel the lint.
E.g. when there's a `let _: fn(&mut i32) = path_to_fn_ref_mut_i32;` statement somewhere in the crate, we shouldn't suggest removing the mutable reference in the function signature.
It was doing a whole pass through the crate at the end, which seems unnecessary.
It seems like we should be able to add entries to the map in the same lint pass.
The map is untouched all the way until `check_crate_post` (at which point it will be populated by the visitor and finally checked), so it doesn't seem like this changes behavior: it will only be fully populated by the time we reach `check_crate_post` no matter what.
I don't think this will have a significant perf impact but it did show up in a profile with 0.5% for a crate I was looking into and looked like a low hanging fruit.
changelog: none
Prefixing a variable with a `_` does not mean that it will not be used.
If such a variable is used later, do not warn about the fact that its
initialization does not have a side effect as this is fine.
Fix error warning span for issue12045
fixes [Issue#12045](https://github.com/rust-lang/rust-clippy/issues/12045)
In issue#12045, unexpected warning span occurs on attribute `#[derive(typed_builder::TypedBuilder)]`, actually the warning should underline `_lifetime`.
In the source code we can find that the original intend is to warning on `ident.span`, but in this case, `stmt.span` is unequal with `ident.span`. So, fix the nit here is fine.
Besides, `ident.span` have an accurate range than `stmt.span`.
changelog: [`no_effect_underscore_binding`]: correct warning span
fix FP on [`semicolon_if_nothing_returned`]
fixes: #12123
---
changelog: fix FP on [`semicolon_if_nothing_returned`] which suggesting adding semicolon after attr macro
Move async closure parameters into the resultant closure's future eagerly
Move async closure parameters into the closure's resultant future eagerly.
Before, we used to desugar `async |p1, p2, ..| { body }` as `|p1, p2, ..| { || async { body } }`. Now, we desugar the above like `|p1, p2, ..| { async move { let p1 = p1; let p2 = p2; ... body } }`. This mirrors the same desugaring that `async fn` does with its parameter types, and the compiler literally uses the same code via a shared helper function.
This removes the necessity for E0708, since now expressions like `async |x: i32| { x }` will not give you confusing borrow errors.
This does *not* fix the case where async closures have self-borrows. This will come with a general implementation of async closures, which is still in the works.
r? oli-obk
Fix [`multiple_crate_versions`] to correctly normalize package names to avoid missing the local one
Fixes#12145
changelog: [`multiple_crate_versions`]: correctly normalize package name
Correctly handle type relative in trait_duplication_in_bounds lint
Fixes#9961.
The generic bounds were not correctly checked and left out `QPath::TypeRelative`, making different bounds look the same and generating invalid errors (and fix).
r? `@blyxyas`
changelog: [`trait_duplication_in_bounds`]: Correctly handle type relative.
`read_zero_byte_vec` refactor for better heuristics
Fixes#9274
Previously, the implementation of `read_zero_byte_vec` only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor.
This PR refactors so that:
1. It checks if there is a `resize` on the vec
2. It works on blocks properly
e.g. This should properly lint now:
```
let mut v = Vec::new();
{
f.read(&mut v)?;
//~^ ERROR: reading zero byte data to `Vec`
}
```
changelog: [`read_zero_byte_vec`] Refactored for better heuristics
Add suspicious_open_options lint.
changelog: [`suspicious_open_options`]: Checks for the suspicious use of std::fs::OpenOptions::create() without an explicit OpenOptions::truncate().
create() alone will either create a new file or open an existing file. If the file already exists, it will be overwritten when written to, but the file will not be truncated by default. If less data is written to the file than it already contains, the remainder of the file will remain unchanged, and the end of the file will contain old data.
In most cases, one should either use `create_new` to ensure the file is created from scratch, or ensure `truncate` is called so that the truncation behaviour is explicit. `truncate(true)` will ensure the file is entirely overwritten with new data, whereas `truncate(false)` will explicitely keep the default behavior.
```rust
use std::fs::OpenOptions;
OpenOptions::new().create(true).truncate(true);
```
- [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`
Try to improve wording and fix dead link in description of arc_with_non_send_sync lint.
changelog: [`arc_with_non_send_sync`]: Improve wording and fix dead link.
Correctly suggest std or core path depending if this is a `no_std` crate
A few lints emit suggestions using `std` paths whether or not this is a `no_std` crate, which is an issue when running `rustfix` afterwards. So in case this is an item that is defined in both `std` and `core`, we need to check if the crate is `no_std` to emit the right path.
r? `@llogiq`
changelog: Correctly suggest std or core path depending if this is a `no_std` crate
- New ineffective_open_options had to be fixed.
- Now not raising an issue on missing `truncate` when `append(true)`
makes the intent clear.
- Try implementing more advanced tests for non-chained operations. Fail
Checks for the suspicious use of OpenOptions::create()
without an explicit OpenOptions::truncate().
create() alone will either create a new file or open an
existing file. If the file already exists, it will be
overwritten when written to, but the file will not be
truncated by default. If less data is written to the file
than it already contains, the remainder of the file will
remain unchanged, and the end of the file will contain old
data.
In most cases, one should either use `create_new` to ensure
the file is created from scratch, or ensure `truncate` is
called so that the truncation behaviour is explicit.
`truncate(true)` will ensure the file is entirely overwritten
with new data, whereas `truncate(false)` will explicitely
keep the default behavior.
```rust
use std::fs::OpenOptions;
OpenOptions::new().create(true).truncate(true);
```
[`useless_asref`]: check that the clone receiver is the parameter
Fixes#12135
There was no check for the receiver of the `clone` call in the map closure. This makes sure that it's a path to the parameter.
changelog: [`useless_asref`]: check that the clone receiver is the closure parameter
['arc_with_non_send_sync`] documentation edits
Arc's documentation uses the term "thread"; aligning to that terminology. Fix casing of "Rc".
changelog: None
There are cases where the closure call is needed in some macros, this in
particular occurs when the closure has parameters. To handle this case,
we allow the lint when there are no parameters in the closure, or the
closure is outside a macro invocation.
fixes: #11274, #1553
changelog: FP: [`redundant_closure_call`] when closures with parameters
are passed in macros.
Fix false positive in `PartialEq` check in `unconditional_recursion` lint
Fixes https://github.com/rust-lang/rust-clippy/issues/12133.
We needed to check for the type of the previous element <del>in case it's a field</del>.
EDIT: After some extra thoughts, no need to check if it's a field, just if it's the same type as `Self`.
r? `@llogiq`
changelog: Fix false positive in `PartialEq` check in `unconditional_recursion` lint
Fix suggestion for `map_clone` lint on types implementing `Copy`
Follow-up of https://github.com/rust-lang/rust-clippy/pull/12104.
It was missing this check to suggest the correct method.
r? `@llogiq`
changelog: Fix suggestion for `map_clone` lint on types implementing `Copy`
One consequence is that errors returned by
`maybe_new_parser_from_source_str` now must be consumed, so a bunch of
places that previously ignored those errors now cancel them. (Most of
them explicitly dropped the errors before. I guess that was to indicate
"we are explicitly ignoring these", though I'm not 100% sure.)
