Separate lifetime ident from lifetime resolution in HIR
Drive-by: change how suggested generic args are computed.
Fixes https://github.com/rust-lang/rust/issues/103815
I recommend reviewing commit-by-commit.
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 😢
`Res::SelfTy` currently has two `Option`s. When the second one is `Some`
the first one is never consulted. So we can split it into two variants,
`Res::SelfTyParam` and `Res::SelfTyAlias`, reducing the size of `Res`
from 24 bytes to 12. This then shrinks `hir::Path` and
`hir::PathSegment`, which are the HIR types that take up the most space.
Stabilize const `BTree{Map,Set}::new`
The FCP was completed in #71835.
Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
Implement https://github.com/rust-lang/rust-clippy/issues/8368 - a new
lint to inline format arguments such as `print!("{}", var)` into
`print!("{var}")`.
code | suggestion | comment
---|---|---
`print!("{}", var)` | `print!("{var}")` | simple variables
`print!("{0}", var)` | `print!("{var}")` | positional variables
`print!("{v}", v=var)` | `print!("{var}")` | named variables
`print!("{0} {0}", var)` | `print!("{var} {var}")` | aliased variables
`print!("{0:1$}", var, width)` | `print!("{var:width$}")` | width
support
`print!("{0:.1$}", var, prec)` | `print!("{var:.prec$}")` | precision
support
`print!("{:.*}", prec, var)` | `print!("{var:.prec$}")` | asterisk
support
code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined. Supporting this
case requires re-indexing of the format string.
changelog: [`uninlined_format_args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
fix a ui test
use `into`
fix clippy ui test
fix a run-make-fulldeps test
implement `IntoQueryParam<DefId>` for `OwnerId`
use `OwnerId` for more queries
change the type of `ParentOwnerIterator::Item` to `(OwnerId, OwnerNode)`
Since clippy can use a projects MSRV for its lints, it might not want
to consider functions as const stable if they have been added lately.
Functions that have been stabilized this version use
CURRENT_RUSTC_VERSION as their version, which gets then turned into the
current version, which might be something like `1.66.0-dev`. The version
parser cannot deal with this version, so it has to be stripped off.
Initial implementation of dyn*
This PR adds extremely basic and incomplete support for [dyn*](https://smallcultfollowing.com/babysteps//blog/2022/03/29/dyn-can-we-make-dyn-sized/). The goal is to get something in tree behind a flag to make collaboration easier, and also to make sure the implementation so far is not unreasonable. This PR does quite a few things:
* Introduce `dyn_star` feature flag
* Adds parsing for `dyn* Trait` types
* Defines `dyn* Trait` as a sized type
* Adds support for explicit casts, like `42usize as dyn* Debug`
* Including const evaluation of such casts
* Adds codegen for drop glue so things are cleaned up properly when a `dyn* Trait` object goes out of scope
* Adds codegen for method calls, at least for methods that take `&self`
Quite a bit is still missing, but this gives us a starting point. Note that this is never intended to become stable surface syntax for Rust, but rather `dyn*` is planned to be used as an implementation detail for async functions in dyn traits.
Joint work with `@nikomatsakis` and `@compiler-errors.`
r? `@bjorn3`
Migrate write.rs to a late pass
changelog: Migrates write.rs from a pre expansion pass to a late pass
changelog: [`positional_named_format_parameters`] is renamed in favour of the rustc lint `named_arguments_used_positionally`
- Macros are now identified by diagnostic items, so will no longer lint user defined macros named, e.g. a custom `print!`
- `print_literal`/`write_literal` no longer lint no longer lint literals that come from macro expansions, e.g. `env!("FOO")`
- `print_with_newline`/`write_with_newline` no longer lint strings with any internal `\r` or `\n`s
~~A false negative, `print_literal`/`write_literal` don't lint format strings that produce `FormatSpec`s, e.g. ones containing pretty print/width/align specifiers~~
Suggestion changes:
- ~~`print_literal`/`write_literal` no longer have suggestions, as the spans for the `{}`s were not easily obtainable~~
- `print_with_newline`/`write_with_newline` has a better suggestion for a sole literal newline, but no longer has suggestions for len > 1 strings that end in a literal newline
- ~~`use_debug` spans are less precise, now point to the whole format string~~
The diff for write.rs is pretty unwieldy, other than for the `declare_clippy_lint!`s I think you'd be better off viewing it as a brand new file rather than looking at the diff, as it's mostly written from scratch
cc #6610, fixes#5721, fixes#7195, fixes#8615
Don't panic on invalid shift while constfolding
Instead of panicking on invalid shifts while folding constants we simply give up. Fixes#9463
Notice the "attempt to shift right by `1316134912_u32`", which seems weird. AFAICS it comes from rustc itself.
changelog: none
Use macro callsite when creating `Sugg` helper
Closes#9375
changelog: Improvement: [`collapsible_if`]: Suggestions now work with macros, by taking the call site into account.
This shrinks `hir::Ty` from 72 to 48 bytes.
`visit_lifetime` is added to the HIR stats collector because these types
are now stored in memory on their own, instead of being within other
types.
`BindingAnnotation` refactor
* `ast::BindingMode` is deleted and replaced with `hir::BindingAnnotation` (which is moved to `ast`)
* `BindingAnnotation` is changed from an enum to a tuple struct e.g. `BindingAnnotation(ByRef::No, Mutability::Mut)`
* Associated constants added for convenience `BindingAnnotation::{NONE, REF, MUT, REF_MUT}`
One goal is to make it more clear that `BindingAnnotation` merely represents syntax `ref mut` and not the actual binding mode. This was especially confusing since we had `ast::BindingMode`->`hir::BindingAnnotation`->`thir::BindingMode`.
I wish there were more symmetry between `ByRef` and `Mutability` (variant) naming (maybe `Mutable::Yes`?), and I also don't love how long the name `BindingAnnotation` is, but this seems like the best compromise. Ideas welcome.
`explicit_auto_deref` changes
fixes#9123fixes#9109fixes#9143fixes#9101
This avoid suggesting code which hits a rustc bug. Basically `&{x}` won't use auto-deref if the target type is `Sized`.
changelog: Don't suggest using auto deref for block expressions when the target type is `Sized`
changelog: Include the borrow in the suggestion for `explicit_auto_deref`
changelog: Don't lint `explicit_auto_deref` on `dyn Trait` return
changelog: Don't lint `explicit_auto_deref` when other adjustments are required
changelog: Lint `explicit_auto_deref` in implicit return positions for closures
Fix suggestions for `async` closures in redundant_closure_call
Fixes#9052
changelog: Fix suggestions given by [`redundant_closure_call`] for async closures
From 72 bytes to 12 bytes (on x86-64).
There are two parts to this:
- Changing various source code offsets from 64-bit to 32-bit. This is
not a problem because the rest of rustc also uses 32-bit source code
offsets. This means `Token` is no longer `Copy` but this causes no
problems.
- Removing the `RawStrError` from `LiteralKind`. Raw string literal
invalidity is now indicated by a `None` value within
`RawStr`/`RawByteStr`, and the new `validate_raw_str` function can be
used to re-lex an invalid raw string literal to get the `RawStrError`.
There is one very small change in behaviour. Previously, if a raw string
literal matched both the `InvalidStarter` and `TooManyHashes` cases,
the latter would override the former. This has now changed, because
`raw_double_quoted_string` now uses `?` and so returns immediately upon
detecting the `InvalidStarter` case. I think this is a slight
improvement to report the earlier-detected error, and it explains the
change in the `test_too_many_hashes` test.
The commit also removes a couple of comments that refer to #77629 and
say that the size of these types don't affect performance. These
comments are wrong, though the performance effect is small.
Implement `for<>` lifetime binder for closures
This PR implements RFC 3216 ([TI](https://github.com/rust-lang/rust/issues/97362)) and allows code like the following:
```rust
let _f = for<'a, 'b> |a: &'a A, b: &'b B| -> &'b C { b.c(a) };
// ^^^^^^^^^^^--- new!
```
cc ``@Aaron1011`` ``@cjgillot``
Fixes for `branches_sharing_code`
fixes#7198fixes#7452fixes#7555fixes#7589
changelog: Don't suggest moving modifications to locals used in any of the condition expressions in `branches_sharing_code`
changelog: Don't suggest moving anything after a local with a significant drop in `branches_sharing_code`
* Don't suggest moving modifications to locals used in any of the condition expressions
* Don't suggest moving anything after a local with a significant drop
rustc comiler internals helpfully tell us how to fix the issue:
to get the signature of a closure, use `substs.as_closure().sig()` not `fn_sig()`
Fixes ICE in #9041
Fix `#[expect]` for most clippy lints
This PR fixes most `#[expect]` - lint interactions listed in rust-lang/rust#97660. [My comment in the issue](https://github.com/rust-lang/rust/issues/97660#issuecomment-1147269504) shows the current progress (Once this is merged). I plan to work on `duplicate_mod` and `multiple_inherent_impl` and leave the rest for later. I feel like stabilizing the feature is more important than fixing the last few nits, which currently also don't work with `#[allow]`.
---
changelog: none
r? `@Jarcho`
cc: rust-lang/rust#97660
feat(new lint): new lint `manual_retain`
close#8097
This PR is a new lint implementation.
This lint checks if the `retain` method is available.
Thank you in advance.
changelog: add new ``[`manual_retain`]`` lint
feat(fix): ignore `todo!` and `unimplemented!` in `if_same_then_else`
close: #8836
take over: #8853
This PR adds check `todo!` and `unimplemented!` in if_same_then_else.
( I thought `unimplemented` should not be checked as well as todo!.)
Thank you in advance.
changelog: ignore todo! and unimplemented! in if_same_then_else
r? `@Jarcho`
once cell renamings
This PR does the renamings proposed in https://github.com/rust-lang/rust/issues/74465#issuecomment-1153703128
- Move/rename `lazy::{OnceCell, Lazy}` to `cell::{OnceCell, LazyCell}`
- Move/rename `lazy::{SyncOnceCell, SyncLazy}` to `sync::{OnceLock, LazyLock}`
(I used `Lazy...` instead of `...Lazy` as it seems to be more consistent, easier to pronounce, etc)
```@rustbot``` label +T-libs-api -T-libs
feat(lint): add default_iter_empty
close#8915
This PR adds `default_iter_empty` lint.
This lint checks `std::iter::Empty::default()` and replace with `std::iter::empty()`.
Thank you in advance.
---
changelog: add `default_instead_of_iter_empty` lint.
Update description in clippy_lints/src/default_iter_empty.rs
Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>
Update clippy_lints/src/default_iter_empty.rs
Co-authored-by: Alex Macleod <alex@macleod.io>
Update clippy_lints/src/default_iter_empty.rs
Co-authored-by: Alex Macleod <alex@macleod.io>
renamed default_iter_empty to default_instead_of_iter_empty
Avoid duplicate messages
add tests for regression
rewrite 'Why is this bad?'
cargo dev fmt
delete default_iter_empty lint in renamed_lint.rs
rewrite a message in the suggestion
cargo dev update_lints --check
Make `ExprKind::Closure` a struct variant.
Simple refactor since we both need it to introduce additional fields in `ExprKind::Closure`.
r? ``@Aaron1011``
Rework `branches_sharing_code`
fixes#7378
This changes the lint from checking pairs of blocks, to checking all the blocks at the same time. As such there's almost none of the original code left.
changelog: Don't lint `branches_sharing_code` when using different binding names
And likewise for the `Const::val` method.
Because its type is called `ConstKind`. Also `val` is a confusing name
because `ConstKind` is an enum with seven variants, one of which is
called `Value`. Also, this gives consistency with `TyS` and `PredicateS`
which have `kind` fields.
The commit also renames a few `Const` variables from `val` to `c`, to
avoid confusion with the `ConstKind::Value` variant.
* Don't lint on `.cloned().flatten()` when `T::Item` doesn't implement `IntoIterator`
* Reduce verbosity of lint message
* Narrow down the scope of the replacement range
Compute `is_late_bound_map` query separately from lifetime resolution
This query is actually very simple, and is only useful for functions and method. It can be computed directly by fetching the HIR, with no need to embed it within the lifetime resolution visitor.
Based on https://github.com/rust-lang/rust/pull/96296
Lazify `SourceFile::lines`.
`SourceFile::lines` is a big part of metadata. It's stored in a compressed form
(a difference list) to save disk space. Decoding it is a big fraction of
compile time for very small crates/programs.
This commit introduces a new type `SourceFileLines` which has a `Lines`
form and a `Diffs` form. The latter is used when the metadata is first
read, and it is only decoded into the `Lines` form when line data is
actually needed. This avoids the decoding cost for many files,
especially in `std`. It's a performance win of up to 15% for tiny
crates/programs where metadata decoding is a high part of compilation
costs.
A `RefCell` is needed because the methods that access lines data (which can
trigger decoding) take `&self` rather than `&mut self`. To allow for this,
`SourceFile::lines` now takes a `FnMut` that operates on the lines slice rather
than returning the lines slice.
r? `@Mark-Simulacrum`
When setting suggestion for significant_drop_in_scrutinee, add suggestion for MoveAndClone for non-ref
When trying to set the current suggestion, if the type of the expression
is not a reference and it is not trivially pure clone copy, we should still
trigger and emit a lint message. Since this fix may require cloning an
expensive-to-clone type, do not attempt to offer a suggested fix.
This change means that matches generated from TryDesugar and AwaitDesugar
would normally trigger a lint, but they are out of scope for this lint,
so we will explicitly ignore matches with sources of TryDesugar or
AwaitDesugar.
changelog: Update for ``[`significant_drop_in_scrutinee`]`` to correctly
emit lint messages for cases where the type is not a reference *and*
not trivially pure clone copy.
changelog: [`significant_drop_in_scrutinee`]: No longer lint on Try `?`
and `await` desugared expressions.
Set correct `ParamEnv` for `derive_partial_eq_without_eq`
fixes#8867
changelog: Handle differing predicates applied by `#[derive(PartialEq)]` and `#[derive(Eq)]` in `derive_partial_eq_without_eq`
new lint: `borrow_deref_ref`
changelog: ``[`borrow_deref_ref`]``
Related pr: #6837#7577
`@Jarcho` Could you please give a review?
`cargo lintcheck` gives no false negative (but tested crates are out-of-date).
TODO:
1. Not sure the name. `deref_on_immutable_ref` or some others?
`SourceFile::lines` is a big part of metadata. It's stored in a compressed form
(a difference list) to save disk space. Decoding it is a big fraction of
compile time for very small crates/programs.
This commit introduces a new type `SourceFileLines` which has a `Lines`
form and a `Diffs` form. The latter is used when the metadata is first
read, and it is only decoded into the `Lines` form when line data is
actually needed. This avoids the decoding cost for many files,
especially in `std`. It's a performance win of up to 15% for tiny
crates/programs where metadata decoding is a high part of compilation
costs.
A `Lock` is needed because the methods that access lines data (which can
trigger decoding) take `&self` rather than `&mut self`. To allow for this,
`SourceFile::lines` now takes a `FnMut` that operates on the lines slice rather
than returning the lines slice.
When trying to set the current suggestion, if the type of the expression
is not a reference and it is not trivially pure clone copy, we should still
trigger and emit a lint message. Since this fix may require cloning an
expensive-to-clone type, do not attempt to offer a suggested fix.
This change means that matches generated from TryDesugar and AwaitDesugar
would normally trigger a lint, but they are out of scope for this lint,
so we will explicitly ignore matches with sources of TryDesugar or
AwaitDesugar.
changelog: Update for [`significant_drop_in_scrutinee`] to correctly
emit lint messages for cases where the type is not a reference and
not trivially pure clone copy.
Refactor call terminator to always include destination place
In #71117 people seemed to agree that call terminators should always have a destination place, even if the call was guaranteed to diverge. This implements that. Unsurprisingly, the diff touches a lot of code, but thankfully I had to do almost nothing interesting. The only interesting thing came up in const prop, where the stack frame having no return place was also used to indicate that the layout could not be computed (or similar). I replaced this with a ZST allocation, which should continue to do the right things.
cc `@RalfJung` `@eddyb` who were involved in the original conversation
r? rust-lang/mir-opt
Lifetime variance fixes for clippy
#97287 migrates rustc to a `Ty` type that is invariant over its lifetime `'tcx`, so I need to fix a bunch of places that assume that `Ty<'a>` and `Ty<'b>` can be shortened to some common lifetime.
This is doable, since everything is already `'tcx`, so all this PR does is be a bit more explicit that elided lifetimes are actually `'tcx`.
Split out from #97287 so the clippy team can review independently.
Drop Tracking: Implement `fake_read` callback
This PR updates drop tracking's use of `ExprUseVisitor` so that we treat `fake_read` events as borrows. Without doing this, we were not handling match expressions correctly, which showed up as a breakage in the `addassign-yield.rs` test. We did not previously notice this because we still had rather large temporary scopes that we held borrows for, which changed in #94309.
This PR also includes a variant of the `addassign-yield.rs` test case to make sure we continue to have correct behavior here with drop tracking.
r? `@nikomatsakis`
Add a query for checking whether a function is an intrinsic.
work towards #93145
This will reduce churn when we add more ways to declare intrinsics
r? `@scottmcm`
Don't lint `vec_init_then_push` when further extended
fixes#7071
This will still lint when a larger number of pushes are done (four currently). The exact number could be debated, but this is more readable then a sequence of pushes so it shouldn't be too large.
changelog: Don't lint `vec_init_then_push` when further extended.
changelog: Remove `mut` binding from `vec_init_then_push` when possible.
Add EarlyBinder
Chalk has no concept of `Param` (e0ade19d13/chalk-ir/src/lib.rs (L579)) or `ReEarlyBound` (e0ade19d13/chalk-ir/src/lib.rs (L1308)). Everything is just "bound" - the equivalent of rustc's late-bound. It's not completely clear yet whether to move everything to the same time of binder in rustc or add `Param` and `ReEarlyBound` in Chalk.
Either way, tracking when we have or haven't already substituted out these in rustc can be helpful.
As a first step, I'm just adding a `EarlyBinder` newtype that is required to call `subst`. I also add a couple "transparent" `bound_*` wrappers around a couple query that are often immediately substituted.
r? `@nikomatsakis`
Replace `#[allow]` with `#[expect]` in Clippy
Hey `@rust-lang/clippy,` `@Alexendoo,` `@dswij,` I'm currently working on the expect attribute as defined in [Rust RFC 2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html). With that, an `#[allow]` attribute can be replaced with a `#[expect]` attribute that suppresses the lint, but also emits a warning, if the lint isn't emitted in the expected scope.
With this PR I would like to test the attribute on a project scale and Clippy obviously came to mind. This PR replaces (almost) all `#[allow]` attributes in `clippy_utils` and `clippy_lints` with the `#[expect]` attribute. I was also able to remove some allows since, the related FPs have been fixed 🎉.
My question is now, are there any concerns regarding this? It's still okay to add normal `#[allow]` attributes, I see the need to nit-pick about that in new PRs, unless it's actually a FP. Also, I would not recommend using `#[expect]` in tests, as changes to a lint could the trigger the expect attribute in other files.
Additionally, I've noticed that Clippy has a bunch of `#[allow(clippy::too_many_lines)]` attributes. Should we maybe allow the lint all together or increase the threshold setting? To me, it seems like we mostly just ignore it in our code. 😅🙃
---
changelog: none
r? `@flip1995` (I've requested you for now, since you're also helping with reviewing the expect implementation. You are welcome to delegate this PR, even if it should be a simple review 🙃 )
Support negative ints in manual_range_contains
fixes: #8721
changelog: Fixes issue where ranges containing ints with different signs would be
incorrect due to comparing as unsigned.
Overhaul `MacArgs`
Motivation:
- Clarify some code that I found hard to understand.
- Eliminate one use of three places where `TokenKind::Interpolated` values are created.
r? `@petrochenkov`
The value in `MacArgs::Eq` is currently represented as a `Token`.
Because of `TokenKind::Interpolated`, `Token` can be either a token or
an arbitrary AST fragment. In practice, a `MacArgs::Eq` starts out as a
literal or macro call AST fragment, and then is later lowered to a
literal token. But this is very non-obvious. `Token` is a much more
general type than what is needed.
This commit restricts things, by introducing a new type `MacArgsEqKind`
that is either an AST expression (pre-lowering) or an AST literal
(post-lowering). The downside is that the code is a bit more verbose in
a few places. The benefit is that makes it much clearer what the
possibilities are (though also shorter in some other places). Also, it
removes one use of `TokenKind::Interpolated`, taking us a step closer to
removing that variant, which will let us make `Token` impl `Copy` and
remove many "handle Interpolated" code paths in the parser.
Things to note:
- Error messages have improved. Messages like this:
```
unexpected token: `"bug" + "found"`
```
now say "unexpected expression", which makes more sense. Although
arbitrary expressions can exist within tokens thanks to
`TokenKind::Interpolated`, that's not obvious to anyone who doesn't
know compiler internals.
- In `parse_mac_args_common`, we no longer need to collect tokens for
the value expression.
Only crate root def-ids don't have a parent, and in majority of cases the argument of `DefIdTree::parent` cannot be a crate root.
So we now panic by default in `parent` and introduce a new non-panicing function `opt_parent` for cases where the argument can be a crate root.
Same applies to `local_parent`/`opt_local_parent`.
`needless_late_init`: ignore `if let`, `let mut` and significant drops
No longer lints `if let`, personal taste on this one is pretty split, so it probably shouldn't be warning by default. Fixes#8613
```rust
let x = if let Some(n) = y {
n
} else {
1
}
```
No longer lints `let mut`, things like the following are not uncommon and look fine as they are
b169c16d86/src/sixty_four.rs (L88-L93)
Avoids changing the drop order in an observable way, where the type of `x` has a drop with side effects and something between `x` and the first use also does, e.g.
48cc6cb791/tests/test_api.rs (L159-L167)
The implementation of `type_needs_ordered_drop_inner` was changed a bit, it now uses `Ty::has_significant_drop` and reordered the ifs to check diagnostic name before checking the implicit drop impl
changelog: [`needless_late_init`]: No longer lints `if let` statements, `let mut` bindings and no longer significantly changes drop order
adding test patterns
cargo dev bless
fix comment
add ;
delete :
fix suggestion code
and update stderr in tests.
use match_def_path when checking method name
Implement sym operands for global_asm!
Tracking issue: #93333
This PR is pretty much a complete rewrite of `sym` operand support for inline assembly so that the same implementation can be shared by `asm!` and `global_asm!`. The main changes are:
- At the AST level, `sym` is represented as a special `InlineAsmSym` AST node containing a path instead of an `Expr`.
- At the HIR level, `sym` is split into `SymStatic` and `SymFn` depending on whether the path resolves to a static during AST lowering (defaults to `SynFn` if `get_early_res` fails).
- `SymFn` is just an `AnonConst`. It runs through typeck and we just collect the resulting type at the end. An error is emitted if the type is not a `FnDef`.
- `SymStatic` directly holds a path and the `DefId` of the `static` that it is pointing to.
- The representation at the MIR level is mostly unchanged. There is a minor change to THIR where `SymFn` is a constant instead of an expression.
- At the codegen level we need to apply the target's symbol mangling to the result of `tcx.symbol_name()` depending on the target. This is done by calling the LLVM name mangler, which handles all of the details.
- On Mach-O, all symbols have a leading underscore.
- On x86 Windows, different mangling is used for cdecl, stdcall, fastcall and vectorcall.
- No mangling is needed on other platforms.
r? `@nagisa`
cc `@eddyb`
New lint `format_add_strings`
Closes#6261
changelog: Added [`format_add_string`]: recommend using `write!` instead of appending the result of `format!`
Check for loops/closures in `local_used_after_expr`
Follow up to #8646, catches when a local is used multiple times because it's in a loop or a closure
changelog: none
Use mir constant in thir instead of ty::Const
This is blocked on https://github.com/rust-lang/rust/pull/94059 (does include its changes, the first two commits in this PR correspond to those changes) and https://github.com/rust-lang/rust/pull/93800 being reinstated (which had to be reverted). Mainly opening since `@lcnr` offered to give some feedback and maybe also for a perf-run (if necessary).
This currently contains a lot of duplication since some of the logic of `ty::Const` had to be copied to `mir::ConstantKind`, but with the introduction of valtrees a lot of that functionality will disappear from `ty::Const`.
Only the last commit contains changes that need to be reviewed here. Did leave some `FIXME` comments regarding future implementation decisions and some things that might be incorrectly implemented.
r? `@oli-obk`
New lint `is_digit_ascii_radix`
Closes#6399
changelog: Added [`is_digit_ascii_radix`]: recommend `is_ascii_digit()` or `is_ascii_hexdigit()` in place of `is_digit(10)` and `is_digit(16)`
Don't lint various match lints when expanded by a proc-macro
fixes#4952
As always for proc-macro output this is a hack-job of a fix. It would be really nice if more proc-macro authors would set spans correctly.
changelog: Don't lint various lints on proc-macro output.
Fix `same_functions_in_if_condition` FP
fixes#8139
changelog: Don't consider `Foo<{ SomeConstant }>` and `Foo<{ SomeOtherConstant }>` to be the same, even if the constants have the same value.
`MultiSpan` contains labels, which are more complicated with the
introduction of diagnostic translation and will use types from
`rustc_errors` - however, `rustc_errors` depends on `rustc_span` so
`rustc_span` cannot use types like `DiagnosticMessage` without
dependency cycles. Introduce a new `rustc_error_messages` crate that can
contain `DiagnosticMessage` and `MultiSpan`.
Signed-off-by: David Wood <david.wood@huawei.com>
This commit makes `AdtDef` use `Interned`. Much the commit is tedious
changes to introduce getter functions. The interesting changes are in
`compiler/rustc_middle/src/ty/adt.rs`.
Currently some `Allocation`s are interned, some are not, and it's very
hard to tell at a use point which is which.
This commit introduces `ConstAllocation` for the known-interned ones,
which makes the division much clearer. `ConstAllocation::inner()` is
used to get the underlying `Allocation`.
In some places it's natural to use an `Allocation`, in some it's natural
to use a `ConstAllocation`, and in some places there's no clear choice.
I've tried to make things look as nice as possible, while generally
favouring `ConstAllocation`, which is the type that embodies more
information. This does require quite a few calls to `inner()`.
The commit also tweaks how `PartialOrd` works for `Interned`. The
previous code was too clever by half, building on `T: Ord` to make the
code shorter. That caused problems with deriving `PartialOrd` and `Ord`
for `ConstAllocation`, so I changed it to build on `T: PartialOrd`,
which is slightly more verbose but much more standard and avoided the
problems.
