Check MethodCall/Call arg count earlier or at all
This gets rid of a bunch of possible panic spots, as well as bailing out earlier for optimisation reasons.
I started doing this because I saw that a significant amount of time was being spent in the `create_dir` restriction lint when running clippy with `perf`, but this also helps with robustness.
changelog: none
Turn declare_clippy_lint into a declarative macro
Ease of development, and hopefully compile times (the dependencies are still there because of ui-test). The procedural macro was doing just some very basic processing (like assigning a lint level to each category), so it didn't have a reason to stay IMO
changelog: None
Don't warn on proc macro generated code in `needless_return`
Fixes#13458Fixes#13457Fixes#13467Fixes#13479Fixes#13481Fixes#13526Fixes#13486
The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does* fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.
The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.
"Hide whitespace" helps a bit for reviewing the proc macro detection change
changelog: none
Don't warn on proc macro generated code in `needless_return`
Fixes#13458Fixes#13457Fixes#13467Fixes#13479Fixes#13481Fixes#13526Fixes#13486
The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does* fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.
The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.
"Hide whitespace" helps a bit for reviewing the proc macro detection change
changelog: none
Show interior mutability chain in `mutable_key_type`
Fixes#10619
Just ran into this myself and I definitely agree it's not very nice to have to manually go through all the types involved to figure out why this happens and to evaluate if this is really a problem (knowing if the field of a struct is something that a hash impl relies on), so this changes the lint to emit notes for each step involved.
changelog: none
Make opaque types regular HIR nodes
Having opaque types as HIR owner introduces all sorts of complications. This PR proposes to make them regular HIR nodes instead.
I haven't gone through all the test changes yet, so there may be a few surprises.
Many thanks to `@camelid` for the first draft.
Fixes https://github.com/rust-lang/rust/issues/129023Fixes#129099Fixes#125843Fixes#119716Fixes#121422
Stabilize the `map`/`value` methods on `ControlFlow`
And fix the stability attribute on the `pub use` in `core::ops`.
libs-api in https://github.com/rust-lang/rust/issues/75744#issuecomment-2231214910 seemed reasonably happy with naming for these, so let's try for an FCP.
Summary:
```rust
impl<B, C> ControlFlow<B, C> {
pub fn break_value(self) -> Option<B>;
pub fn map_break<T>(self, f: impl FnOnce(B) -> T) -> ControlFlow<T, C>;
pub fn continue_value(self) -> Option<C>;
pub fn map_continue<T>(self, f: impl FnOnce(C) -> T) -> ControlFlow<B, T>;
}
```
Resolves#75744
``@rustbot`` label +needs-fcp +t-libs-api -t-libs
---
Aside, in case it keeps someone else from going down the same dead end: I looked at the `{break,continue}_value` methods and tried to make them `const` as part of this, but that's disallowed because of not having `const Drop`, so put it back to not even unstably-const.
Compare trait references in `trait_duplication_in_bounds` correctly
Fixes#13476Fixes#11067Fixes#9915Fixes#9626
Currently, the `trait_duplication_in_bounds` lints has a helper type for a trait reference that can be used for comparison and hashing, represented as `{trait: Res, generic_args: Vec<Res>}`. However, there are a lot of issues with this. For one, a `Res` can't represent e.g. references, slices, or lots of other types, as well as const generics and associated type equality. In those cases, the lint simply ignores them and has no way of checking if they're actually the same.
So, instead of using `Res` for this, use `SpanlessEq` and `SpanlessHash` for comparisons with the trait path for checking if there are duplicates.
However, using `SpanlessEq` as is alone lead to a false negative in the test. `std::clone::Clone` + `foo::Clone` wasn't recognized as a duplicate, because it has different segments. So this also adds a new "mode" to SpanlessEq which compares by final resolution. (I've been wondering if this can't just be the default but it's quite a large scale change as it affects a lot of lints and I haven't yet looked at all uses of it to see if there are lints that really do care about having exactly the same path segments).
Maybe an alternative would be to turn the hir types/consts into middle types/consts and compare them instead but I'm not sure there's really a good way to do that
changelog: none
Refactoring to `OpaqueTyOrigin`
Pulled out of a larger PR that uses these changes to do cross-crate encoding of opaque origin, so we can use them for edition 2024 migrations. These changes should be self-explanatory on their own, tho 😄
Extend `needless_lifetimes` to suggest eliding `impl` lifetimes
Example:
```
error: the following explicit lifetimes could be elided: 'a
--> tests/ui/needless_lifetimes.rs:332:10
|
LL | impl<'a> Foo for Baz<'a> {}
| ^^ ^^
|
help: elide the lifetimes
|
LL - impl<'a> Foo for Baz<'a> {}
LL + impl Foo for Baz<'_> {}
```
The main change is in how `impl` lifetime uses are tracked. Previously, a hashmap was created, and lifetimes were removed from the hashmap as their uses were discovered. However, the uses are needed to generate elision suggestions. So, now, uses are added to the hashmap as they are discovered.
The PR is currently organized as six commits, which I think are self-explanatory:
- Extend `needless_lifetimes` to suggest eliding `impl` lifetimes
- Reorder functions _[not strictly necessary, but IMHO, the code is better structured as a result]_
- Fix lifetime tests
- Fix non-lifetime tests
- Fix `clippy_lints` and `clippy_utils`
- Fix typo in `needless_lifetimes` test
r? `@Alexendoo` (I think you are `needless_lifetimes`' primary author? Sorry if I have this wrong.)
---
changelog: Extend `needless_lifetimes` to suggest eliding `impl` lifetimes
Clippy subtree update
r? `@Manishearth`
Really delayed sync (2 1/2 weeks), because of a `debug_assertion` we hit, and I didn't have the time to investigate earlier.
It would be nice to merge this PR with some priority, as it includes a lot of formatting changes due to the rustfmt bump.
Include Cargo.lock update due to Clippy version bump and ui_test bump in Clippy.
Separate collection of crate-local inherent impls from error tracking
#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until #121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.
Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in #127798.
This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.
This fixes#127798.
check std::panic::panic_any in panic lint
close#13292
This PR detects `std::panic::panic_any` in panic lint.
changelog: check std::panic::panic_any in panic lint
Rewrite `empty_line_after_doc_comments` and `empty_line_after_outer_attr`, move them from `nursery` to `suspicious`
changelog: [`empty_line_after_doc_comments`], [`empty_line_after_outer_attr`]: rewrite and move them from `nursery` to `suspicious`
They now lint when there's a comment between the last attr/doc comment and the empty line, to cover the case:
```rust
/// Docs for `old_code
// fn old_code() {}
fn new_code() {}
```
When these lints or `suspicious_doc_comments` trigger we no longer trigger any other doc lint as a broad fix for #12917, reverts some of #13002 as the empty line lints cover it
I ended up not doing https://github.com/rust-lang/rust-clippy/issues/12917#issuecomment-2161828859 as I don't think it's needed
Remove `find_format_arg_expr` AST fallback
If the function fails where it shouldn't we can fix that directly, but the fallback path is untested as I'm not aware of a case where it would fail
changelog: none
Shrink `TyKind::FnPtr`.
By splitting the `FnSig` within `TyKind::FnPtr` into `FnSigTys` and `FnHeader`, which can be packed more efficiently. This reduces the size of the hot `TyKind` type from 32 bytes to 24 bytes on 64-bit platforms. This reduces peak memory usage by a few percent on some benchmarks. It also reduces cache misses and page faults similarly, though this doesn't translate to clear cycles or wall-time improvements on CI.
r? `@compiler-errors`
Replace `rustc_semver` with `rustc_session::RustcVersion`
Allows dropping a dependency but there is a behaviour change here, the following versions are no longer accepted:
* `1` (would need to be `1.0` or `1.0.0`)
* `1.0.0-alpha`
* `1.0.0-alpha.2`
* `1.0.0-beta`
But this seems unlikely to effect anybody, I did some GitHub searching and found no occurrences outside our UI tests
r? `@flip1995`
changelog: none
Miscellaneous improvements to struct tail normalization
1. Make checks for foreign tails more accurate by normalizing the struct tail. I didn't write a test for this one.
2. Normalize when computing struct tail for `offset_of` for slice/str. This fixes the new solver only.
3. Normalizing when computing tails for disaligned reference check. This fixes both solvers.
r? lcnr
By splitting the `FnSig` within `TyKind::FnPtr` into `FnSigTys` and
`FnHeader`, which can be packed more efficiently. This reduces the size
of the hot `TyKind` type from 32 bytes to 24 bytes on 64-bit platforms.
This reduces peak memory usage by a few percent on some benchmarks. It
also reduces cache misses and page faults similarly, though this doesn't
translate to clear cycles or wall-time improvements on CI.
Don't use `LateContext` in the constant evaluator
This also changes the interface to require explicitly creating the context. `constant` could be added back in, but the others are probably not worth it.
A couple of bugs have been fixed. The wrong `TypeckResults` was used once when evaluating a constant, and the wrong `ParamEnv` was used by some callers (there wasn't a way to use the correct one).
changelog: none
