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.
This internal lint checks if the `extract_msrv_attrs!` macro is used if
a lint has a MSRV. If not, it suggests to add this attribute to the lint
pass implementation.
Specifically, rename the `Const` struct as `ConstS` and re-introduce `Const` as
this:
```
pub struct Const<'tcx>(&'tcx Interned<ConstS>);
```
This now matches `Ty` and `Predicate` more closely, including using
pointer-based `eq` and `hash`.
Notable changes:
- `mk_const` now takes a `ConstS`.
- `Const` was copy, despite being 48 bytes. Now `ConstS` is not, so need a
we need separate arena for it, because we can't use the `Dropless` one any
more.
- Many `&'tcx Const<'tcx>`/`&Const<'tcx>` to `Const<'tcx>` changes
- Many `ct.ty` to `ct.ty()` and `ct.val` to `ct.val()` changes.
- Lots of tedious sigil fiddling.
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
The to_string_in_display lint is renamed to recursive_format_impl
A check is added for the use of self formatted with Display or Debug
inside any format string in the same impl
The to_string_in_display check is kept as is - like in the
format_in_format_args lint
For now only Display and Debug are checked
This could also be extended to other Format traits (Binary, etc.)
Factor out several utils, add `path_def_id`
changelog: none
This is generally an effort to reduce the total number of utils. `path_def_id` is added which I believe is more "cross-cutting" and also complements `path_to_local`. Best reviewed one commit at a time.
Added:
* `path_def_id`
* `path_res`
Removed:
* `is_qpath_def_path`
* `match_any_diagnostic_items`
* `expr_path_res`
* `single_segment_path`
* `differing_macro_contexts`
* `is_ty_param_lang_item`
* `is_ty_param_diagnostic_item`
* `get_qpath_generics`
Renamed:
* `path_to_res` to `def_path_res`
* `get_qpath_generic_tys` to `qpath_generic_tys`
CC `@Jarcho` since this relates to some of your work and you may have input.
by using an opaque type obligation to bubble up comparisons between opaque types and other types
Also uses proper obligation causes so that the body id works, because out of some reason nll uses body ids for logic instead of just diagnostics.
Create `core::fmt::ArgumentV1` with generics instead of fn pointer
Split from (and prerequisite of) #90488, as this seems to have perf implication.
`@rustbot` label: +T-libs
Fix `needless_borrow` causing mutable borrows to be moved
fixes#8191
changelog: Fix `needless_borrow` causing mutable borrows to be moved
changelog: Rename `ref_in_deref` to `needless_borrow`
changelog: Suggest removing the borrow on method call receivers in `needless_borrow`
Check usages in `ptr_arg`
fixes#214fixes#1981fixes#3381fixes#6406fixes#6964
This does not take into account the return type of the function currently, so `(&Vec<_>) -> &Vec<_>` functions may still be false positives.
The name given for the type also has to match the real type name, so `type Foo = Vec<u32>` won't trigger the lint, but `type Vec = Vec<u32>` will. I'm not sure if this is the best way to handle this, or if a note about the actual type should be added instead.
changelog: Check if the argument is used in a way which requires the original type in `ptr_arg`
changelog: Lint mutable references in `ptr_arg`
Replace `NestedVisitorMap` with generic `NestedFilter`
This is an attempt to make the `intravisit::Visitor` API simpler and "more const" with regard to nested visiting.
With this change, `intravisit::Visitor` does not visit nested things by default, unless you specify `type NestedFilter = nested_filter::OnlyBodies` (or `All`). `nested_visit_map` returns `Self::Map` instead of `NestedVisitorMap<Self::Map>`. It panics by default (unreachable if `type NestedFilter` is omitted).
One somewhat trixty thing here is that `nested_filter::{OnlyBodies, All}` live in `rustc_middle` so that they may have `type Map = map::Map` and so that `impl Visitor`s never need to specify `type Map` - it has a default of `Self::NestedFilter::Map`.
Remove deprecated LLVM-style inline assembly
The `llvm_asm!` was deprecated back in #87590 1.56.0, with intention to remove
it once `asm!` was stabilized, which already happened in #91728 1.59.0. Now it
is time to remove `llvm_asm!` to avoid continued maintenance cost.
Closes#70173.
Closes#92794.
Closes#87612.
Closes#82065.
cc `@rust-lang/wg-inline-asm`
r? `@Amanieu`
Closure capture cleanup & refactor
Follow up of #89648
Each commit is self-contained and the rationale/changes are documented in the commit message, so it's advisable to review commit by commit.
The code is significantly cleaner (at least IMO), but that could have some perf implication, so I'd suggest a perf run.
r? `@wesleywiser`
cc `@arora-aman`
* Track the argument when used to initialize simple `let` bindings
* Check if the argument is passed to a function requiring the original type
* Use `multipart_suggestion` rather than multiple suggestions
* Check if the name given in the source code matches the name of the actual type
Some test code cleanup
changelog: none
Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.
Region info is completely unnecessary for upvar capture kind computation
and is only needed to create the final upvar tuple ty. Doing so makes
creation of UpvarCapture very cheap and expose further cleanup opportunity.
Fix `type_repetition_in_bounds`
fixes#7360fixes#8162fixes#8056
changelog: Check for full equality in `type_repetition_in_bounds` rather than just equal hashes
Remove in_macro from clippy_utils
changelog: none
Previously done in #7897 but reverted in #8170. I'd like to keep `in_macro` out of utils because if a span is from expansion in any way (desugaring or macro), we should not proceed without understanding the nature of the expansion IMO.
r? `@llogiq`
New macro utils
changelog: none
Sorry, this is a big one. A lot of interrelated changes and I wanted to put the new utils to use to make sure they are somewhat battle-tested. We may want to divide some of the lint-specific refactoring commits into batches for smaller reviewing tasks. I could also split into more PRs.
Introduces a bunch of new utils at `clippy_utils::macros::...`. Please read through the docs and give any feedback! I'm happy to introduce `MacroCall` and various functions to retrieve an instance. It feels like the missing puzzle piece. I'm also introducing `ExpnId` from rustc as "useful for Clippy too". `@rust-lang/clippy`
Fixes#7843 by not parsing every node of macro implementations, at least the major offenders.
I probably want to get rid of `is_expn_of` at some point.
changelog: none
Sorry, this is a big one. A lot of interrelated changes and I wanted to put the new utils to use to make sure they are somewhat battle-tested. We may want to divide some of the lint-specific refactoring commits into batches for smaller reviewing tasks. I could also split into more PRs.
Introduces a bunch of new utils at `clippy_utils::macros::...`. Please read through the docs and give any feedback! I'm happy to introduce `MacroCall` and various functions to retrieve an instance. It feels like the missing puzzle piece. I'm also introducing `ExpnId` from rustc as "useful for Clippy too". `@rust-lang/clippy`
Fixes#7843 by not parsing every node of macro implementations, at least the major offenders.
I probably want to get rid of `is_expn_of` at some point.
Remove `NullOp::Box`
Follow up of #89030 and MCP rust-lang/compiler-team#460.
~1 month later nothing seems to be broken, apart from a small regression that #89332 (1aac85bb716c09304b313d69d30d74fe7e8e1a8e) shows could be regained by remvoing the diverging path, so it shall be safe to continue and remove `NullOp::Box` completely.
r? `@jonas-schievink`
`@rustbot` label T-compiler
Clippy helpfully warns about code like this, telling you that you
probably meant "write_all":
fn say_hi<W:Write>(w: &mut W) {
w.write(b"hello").unwrap();
}
This patch attempts to extend the lint so it also covers this
case:
async fn say_hi<W:AsyncWrite>(w: &mut W) {
w.write(b"hello").await.unwrap();
}
(I've run into this second case several times in my own programming,
and so have my coworkers, so unless we're especially accident-prone
in this area, it's probably worth addressing?)
This patch covers the Async{Read,Write}Ext traits in futures-rs,
and in tokio, since both are quite widely used.
changelog: [`unused_io_amount`] now supports AsyncReadExt and AsyncWriteExt.
Limit the ``[`identity_op`]`` lint to integral operands.
changelog: limit ``[`identity_op`]`` to integral operands
In the ``[`identity_op`]`` lint, if the operands are non-integers, then the lint is likely
wrong.
Fix `enum_variants` FP on prefixes that are not camel-case
closes#8090
Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.
This changes how the lint behaves:
1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
cFoo,
cBar,
cBaz,
}
enum Something {
CCall,
CCreate,
CCryogenize,
}
```
2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
PrefixXXX,
PrefixTea,
PrefixCake,
}
```
will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.
changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.
---
(Edited by `@xFrednet` removed some non ascii characters)
cache test item names
This avoids quadratic behavior (collecting all test item names for each `eq_op` instance within the module). However, it invests a good deal of memory to buy this speedup. If that becomes a problem, I may need to change the cache to only store the chain of last visited modules.
This hopefully fixes#8171.
---
changelog: none
This makes sure that the tests in clippy_utils are run in CI.
When looking into this I discovered that two tests were failing and
multiple doc tests were failing. This fixes those tests and enables a
few more doc tests.
Remove `SymbolStr`
This was originally proposed in https://github.com/rust-lang/rust/pull/74554#discussion_r466203544. As well as removing the icky `SymbolStr` type, it allows the removal of a lot of `&` and `*` occurrences.
Best reviewed one commit at a time.
r? `@oli-obk`
Implement let-else type annotations natively
Tracking issue: #87335Fixes#89688, fixes#89807, edit: fixes #89960 as well
As explained in https://github.com/rust-lang/rust/issues/89688#issuecomment-940405082, the previous desugaring moved the let-else scrutinee into a dummy variable, which meant if you wanted to refer to it again in the else block, it had moved.
This introduces a new hir type, ~~`hir::LetExpr`~~ `hir::Let`, which takes over all the fields of `hir::ExprKind::Let(...)` and adds an optional type annotation. The `hir::Let` is then treated like a `hir::Local` when type checking a function body, specifically:
* `GatherLocalsVisitor` overrides a new `Visitor::visit_let_expr` and does pretty much exactly what it does for `visit_local`, assigning a local type to the `hir::Let` ~~(they could be deduplicated but they are right next to each other, so at least we know they're the same)~~
* It reuses the code in `check_decl_local` to typecheck the `hir::Let`, simply returning 'bool' for the expression type after doing that.
* ~~`FnCtxt::check_expr_let` passes this local type in to `demand_scrutinee_type`, and then imitates check_decl_local's pattern checking~~
* ~~`demand_scrutinee_type` (the blindest change for me, please give this extra scrutiny) uses this local type instead of of creating a new one~~
* ~~Just realised the `check_expr_with_needs` was passing NoExpectation further down, need to pass the type there too. And apparently this Expectation API already exists.~~
Some other misc notes:
* ~~Is the clippy code supposed to be autoformatted? I tried not to give huge diffs but maybe some rustfmt changes simply haven't hit it yet.~~
* in `rustc_ast_lowering/src/block.rs`, I noticed some existing `self.alias_attrs()` calls in `LoweringContext::lower_stmts` seem to be copying attributes from the lowered locals/etc to the statements. Is that right? I'm new at this, I don't know.
By changing `as_str()` to take `&self` instead of `self`, we can just
return `&str`. We're still lying about lifetimes, but it's a smaller lie
than before, where `SymbolStr` contained a (fake) `&'static str`!
Stabilize `iter::zip`
Hello all!
As the tracking issue (#83574) for `iter::zip` completed the final commenting period without any concerns being raised, I hereby submit this stabilization PR on the issue.
As the pull request that introduced the feature (#82917) states, the `iter::zip` function is a shorter way to zip two iterators. As it's generally a quality-of-life/ergonomic improvement, it has been integrated into the codebase without any trouble, and has been
used in many places across the rust compiler and standard library since March without any issues.
For more details, I would refer to `@cuviper's` original PR, or the [function's documentation](https://doc.rust-lang.org/std/iter/fn.zip.html).
