Add `clippy_utils::msrv::Msrv` to keep track of the current MSRV
changelog: Fix the scoping of the `#![clippy::msrv]` attribute
Fixes#6920
r? `@Jarcho`
Fix [`unnecessary_lazy_eval`] when type has significant drop
fix for https://github.com/rust-lang/rust-clippy/issues/9427#issuecomment-1295742590
However current implementation gives too many false positive, rending the lint almost useless.
I don't know what's the best way to check if a type has a "significant" drop (in the common meaning, not the internal rustc one, for example Option<(u8, u8)> should not be considered significant)
changelog: Fix [`unnecessary_lazy_eval`] when type has significant drop
Update Clippy
r? `@Manishearth`
Sorry for taking so long. There were so many blockers and so little time. This situation should be mitigated with #104007 in the future.
Rollup of 11 pull requests
Successful merges:
- #103396 (Pin::new_unchecked: discuss pinning closure captures)
- #104416 (Fix using `include_bytes` in pattern position)
- #104557 (Add a test case for async dyn* traits)
- #104559 (Split `MacArgs` in two.)
- #104597 (Probe + better error messsage for `need_migrate_deref_output_trait_object`)
- #104656 (Move tests)
- #104657 (Do not check transmute if has non region infer)
- #104663 (rustdoc: factor out common button CSS)
- #104666 (Migrate alias search result to CSS variables)
- #104674 (Make negative_impl and negative_impl_exists take the right types)
- #104692 (Update test's cfg-if dependency to 1.0)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
`MacArgs` is an enum with three variants: `Empty`, `Delimited`, and `Eq`. It's
used in two ways:
- For representing attribute macro arguments (e.g. in `AttrItem`), where all
three variants are used.
- For representing function-like macros (e.g. in `MacCall` and `MacroDef`),
where only the `Delimited` variant is used.
In other words, `MacArgs` is used in two quite different places due to them
having partial overlap. I find this makes the code hard to read. It also leads
to various unreachable code paths, and allows invalid values (such as
accidentally using `MacArgs::Empty` in a `MacCall`).
This commit splits `MacArgs` in two:
- `DelimArgs` is a new struct just for the "delimited arguments" case. It is
now used in `MacCall` and `MacroDef`.
- `AttrArgs` is a renaming of the old `MacArgs` enum for the attribute macro
case. Its `Delimited` variant now contains a `DelimArgs`.
Various other related things are renamed as well.
These changes make the code clearer, avoids several unreachable paths, and
disallows the invalid values.
Improve spans for RPITIT object-safety errors
No reason why we can't point at the `impl Trait` that causes the object-safety violation.
Also [drive-by: Add is_async fn to hir::IsAsync](c4165f3a96), which touches clippy too.
Return multiple resolutions from `def_path_res`
Changes `def_path_res` to return all the resolutions matching the path rather than the first one (with a namespace hint that covered some cases). This would fix any issues that come up with multiple versions of the same crate being present as they all have the same crate name
It also adds resolution of `impl _ {}` items for local items, and removes struct field resolution as it didn't seem to be used anywhere
I tested it on a local crate and it worked for the multiple crate issue, but I couldn't come up with a test that worked well with `// aux-build`, maybe `// aux-crate` after https://github.com/rust-lang/rust/pull/103266 could work but I'm not sure on that either
changelog: [`disallowed_methods`], [`disallowed_types`], [`disallowed_macros`]: fix path resolution with multiple versions of the same crate
changelog: [`disallowed_methods`]: Resolve methods in `impl`s in the current crate
Instead of `ast::Lit`.
Literal lowering now happens at two different times. Expression literals
are lowered when HIR is crated. Attribute literals are lowered during
parsing.
This commit changes the language very slightly. Some programs that used
to not compile now will compile. This is because some invalid literals
that are removed by `cfg` or attribute macros will no longer trigger
errors. See this comment for more details:
https://github.com/rust-lang/rust/pull/102944#issuecomment-1277476773
[`fn_params_excessive_bools`] Make it possible to allow the lint at the method level
changelog: FP: [`fn_params_excessive_bools`]: `#[allow]` now works on methods
fix https://github.com/rust-lang/rust-clippy/issues/9687
Tested without committing but `#[allow]`ing now works. Also rewrote the lint to be a late lint while at it :)
r? `@xFrednet`
Add `manual_is_ascii_check` lint
Addresses https://github.com/rust-lang/rust-clippy/issues/9290
This PR adds new lint `manual_is_ascii_check`, which detects comparison with ascii ranges using `matches!` macros.
As I mentioned as following in the Issue;
> Yes, that's true. we'll start small and then grow it.
> So I'll try to handle matches! macro with single range as suggested above.
However during writing first version, I was thinking that the changes to support alphabetic and digits will be small patch, so I made a single PR in hope review cost can be reduced.
changelog: add new lint [`manual_is_ascii_check`]
r? `@xFrednet`
Improve `needless_lifetimes`
This PR makes the following improvements to `needless_lifetimes`.
* It fixes the following false negative, where `foo` is flagged but `bar` is not:
```rust
fn foo<'a>(x: &'a u8, y: &'_ u8) {}
fn bar<'a>(x: &'a u8, y: &'_ u8, z: &'_ u8) {}
```
* It flags more cases, generally. Previously, `needless_borrow` required *all* lifetimes to be used only once. With the changes, individual lifetimes are flagged for being used only once, even if not all lifetimes are.
* Finally, it tries to produce more clear error messages.
changelog: fix `needless_lifetimes` false negative involving functions with multiple unnamed lifetimes
changelog: in `needless_lifetimes`, flag individual lifetimes used only once, rather than require all lifetimes to be used only once
changelog: in `needless_lifetimes`, emit "replace with `'_`" warnings only when applicable, and point to a generic argument
Accept `TyCtxt` instead of `TyCtxtAt` in `Ty::is_*` functions
Functions in answer:
- `Ty::is_freeze`
- `Ty::is_sized`
- `Ty::is_unpin`
- `Ty::is_copy_modulo_regions`
This allows to remove a lot of useless `.at(DUMMY_SP)`, making the code a bit nicer :3
r? `@compiler-errors`
spastorino noticed some silly expressions like `item_id.def_id.def_id`.
This commit renames several `def_id: OwnerId` fields as `owner_id`, so
those expressions become `item_id.owner_id.def_id`.
`item_id.owner_id.local_def_id` would be even clearer, but the use of
`def_id` for values of type `LocalDefId` is *very* widespread, so I left
that alone.
make ignored internally mutable types for `mutable-key` configurable
We had some false positives where people would create their own types that had interior mutability unrelated to hash/eq. This addition lets you configure this as e.g. `arc-like-types=["bytes::Bytes"]`
This fixes#5325 by allowing users to specify the types whose innards like `Arc` should be ignored (the generic types are still checked) for the sake of detecting inner mutability.
r? `@Alexendoo`
---
changelog: Allow configuring types to ignore internal mutability in `mutable-key`
add new lint `seek_to_start_instead_of_rewind `
changelog: `seek_to_start_instead_of_rewind`: new lint to suggest using `rewind` instead of `seek` to start
Resolve#8600
We had some false positives where people would create their own types
that had interior mutability unrelated to hash/eq. This addition lets
you configure this as e.g. `arc-like-types=["bytes::Bytes"]`
Fix bug introduced by #9386#9386 introduced a potential out-of-bounds array access. Specifically, a location returned by `local_assignments` could have [`location.statement_index` equal to `mir.basic_blocks[location.block].statements.len()`](b8a9a507bf/clippy_utils/src/mir/mod.rs (L129)), in which case the location would refer to the block terminator:
b8a9a507bf/clippy_lints/src/dereference.rs (L1204-L1206)
I suspect the bug is not triggerable now, because of checks leading up to where it occurs. But a future code change could make it triggerable. Hence, it should be fixed.
r? `@Jarcho`
changelog: none
Add `unused_format_specs` lint
Currently catches two cases:
An empty precision specifier:
```rust
// the same as {}
println!("{:.}", x);
```
And using formatting specs on `format_args!()`:
```rust
// prints `x.`, not `x .`
println("{:5}.", format_args!("x"));
```
changelog: new lint: [`unused_format_specs`]
[`unnecessary_cast`] Do not lint negative hexadecimal literals when cast as floats
fix https://github.com/rust-lang/rust-clippy/issues/9603
changelog: [`unnecessary_cast`] Do not lint negative hexadecimal literals when cast as floats
Don't suggest moving tuple structs with a significant drop to late evaluation
fixes#9608
changelog: Don't suggest moving tuple structs with a significant drop to late evaluation
rename `ImplItemKind::TyAlias` to `ImplItemKind::Type`
The naming of this variant seems inconsistent given that this is not really a "type alias", and the associated type variant for `TraitItemKind` is just called `Type`.
Further enhance `needless_borrow`, mildly refactor `redundant_clone`
This PR does the following:
* Moves some code from `redundant_clone` into a new `clippy_utils` module called `mir`, and wraps that code in a function called `dropped_without_further_use`.
* Relaxes the "is copyable" condition condition from #9136 by also suggesting to remove borrows from values dropped without further use. The changes involve the just mentioned function.
* Separates `redundant_clone` into modules.
Strictly speaking, the last bullet is independent of the others. `redundant_clone` is somewhat hairy, IMO. Separating it into modules makes it slightly less so, by helping to delineate what depends upon what.
I've tried to break everything up into digestible commits.
r? `@Jarcho`
(`@Jarcho` I hope you don't mind.)
changelog: continuation of #9136
Add `manual_filter` lint for `Option`
Share much of its implementation with `manual_map` and should greatly benefit from its previous feedback.
I'm sure it's possible to even more refactor both and would gladly take input on that as well as any clippy idiomatic usage, since this is my first lint addition.
I've added the lint to the complexity section for now, I don't know if every new lint needs to go in nursery first.
The matching could be expanded to more than `Some(<value>)` to lint on arbitrary struct matching inside the `Some` but I've left it like it was for `manual_map` for now. `needless_match::pat_same_as_expr` provides a more generic match example.
close https://github.com/rust-lang/rust-clippy/issues/8822
changelog: Add lint [`manual_filter`] for `Option`
Remove `mir::CastKind::Misc`
As discussed in #97649 `mir::CastKind::Misc` is not clear, this PR addresses that by creating a new enum variant for every valid cast.
r? ````@oli-obk````
extend `box-default` lint, add suggestion
This extends the recently added `box-default` lint to also cover `Box::new(vec![])`, `Box::new(String::from(""))` and `Box::new(Vec::from([]))`. Also the lint now suggests a suitable replacement. I did not find a simple way to check whether the type is fully determined by the outside, so I at least checked for some variations to remove the turbofish in those cases.
---
changelog: none
FormatArgsExpn: Find comma spans and ignore weird proc macro spans
Fixes the following cases:
A missing `, 1` from the `expect_fun_call` suggestion:
```rust
Some(()).expect(&format!("{x} {}", 1));
```
```
warning: use of `expect` followed by a function call
--> t.rs:7:14
|
7 | Some(()).expect(&format!("{x} {}", 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{x} {}"))`
```
The suggestion removing from the comma in the comment rather than the one after the format string:
```rust
println!(
"{}",
// a comment, with a comma in it
x
);
```
```
warning: variables can be used directly in the `format!` string
--> t.rs:9:5
|
9 | / println!(
10 | | "{}",
11 | | // a comment, with a comma in it
12 | | x
13 | | );
| |_____^
|
help: change this to
|
10 ~ "{x}",
11 ~ // a comment
|
```
It also no longer accepts expansions where a format string or argument has a "weird" proc macro span, that is one where the literal/expression it outputs has the span of one of its inputs. Kind of like a `format_args` specific `clippy_utils::is_from_proc_macro`, e.g. `format!(indoc! {" ... "})`
changelog: [`expect_fun_call`]: Fix suggestion for `format!` using captured variables
changelog: [`print_literal`], [`write_literal`], [`uninlined_format_args`]: Fix suggestion when following a comment including a comma
* Check for `const`s and `static`s from external crates
* Check for `LangItem`s
* Handle inherent functions which have the same name as a field
* Also check the following functions:
* `match_trait_method`
* `match_def_path`
* `is_expr_path_def_path`
* `is_qpath_def_path`
* Handle checking for a constructor to a diagnostic item or `LangItem`
Move lint level source explanation to the bottom
So, uhhhhh
r? `@estebank`
## User-facing change
"note: `#[warn(...)]` on by default" and such are moved to the bottom of the diagnostic:
```diff
- = note: `#[warn(unsupported_calling_conventions)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #87678 <https://github.com/rust-lang/rust/issues/87678>
+ = note: `#[warn(unsupported_calling_conventions)]` on by default
```
Why warning is enabled is the least important thing, so it shouldn't be the first note the user reads, IMO.
## Developer-facing change
`struct_span_lint` and similar methods have a different signature.
Before: `..., impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>)`
After: `..., impl Into<DiagnosticMessage>, impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>`
The reason for this is that `struct_span_lint` needs to edit the diagnostic _after_ `decorate` closure is called. This also makes lint code a little bit nicer in my opinion.
Another option is to use `impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>) -> DiagnosticBuilder<'a, ()>` altough I don't _really_ see reasons to do `let lint = lint.build(message)` everywhere.
## Subtle problem
By moving the message outside of the closure (that may not be called if the lint is disabled) `format!(...)` is executed earlier, possibly formatting `Ty` which may call a query that trims paths that crashes the compiler if there were no warnings...
I don't think it's that big of a deal, considering that we move from `format!(...)` to `fluent` (which is lazy by-default) anyway, however this required adding a workaround which is unfortunate.
## P.S.
I'm sorry, I do not how to make this PR smaller/easier to review. Changes to the lint API affect SO MUCH 😢
