* 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`)
