Implement new lint `iter_over_hash_type`
Implements and fixes https://github.com/rust-lang/rust-clippy/issues/11788
This PR adds a new *restriction* lint `iter_over_hash_type` which prevents `Hash`-types (that is, `HashSet` and `HashMap`) from being used as the iterator in `for` loops.
The justification for this is because in `Hash`-based types, the ordering of items is not guaranteed and may vary between executions of the same program on the same hardware. In addition, it reduces readability due to the unclear iteration order.
The implementation of this lint also ensures the following:
- Calls to `HashMap::keys`, `HashMap::values`, and `HashSet::iter` are also denied when used in `for` loops,
- When this expression is used in procedural macros, it is not linted/denied.
changelog: add new `iter_over_hash_type` lint to prevent unordered iterations through hashed data structures
Don't check for late-bound vars, check for escaping bound vars
Fixes an assertion that didn't make sense. Many valid and well-formed types *have* late-bound vars (e.g. `for<'a> fn(&'a ())`), they just must not have *escaping* late-bound vars in order to be normalized correctly.
Addresses rust-lang/rust-clippy#11230, cc `@jyn514` and `@matthiaskrgr`
changelog: don't check for late-bound vars, check for escaping bound vars. Addresses rust-lang/rust-clippy#11230
Fixes to `manual_let_else`'s divergence check
A few changes to the divergence check in `manual_let_else` and moves it the implementation to `clippy_utils` since it's generally useful:
* Handle internal `break` and `continue` expressions.
e.g. The first loop is divergent, but the second is not.
```rust
{
loop {
break 'outer;
};
}
{
loop {
break;
};
}
```
* Match rust's definition of divergence which is defined via the type system.
e.g. The following is not considered divergent by rustc as the inner block has a result type of `()`:
```rust
{
'a: {
panic!();
break 'a;
};
}
```
* Handle when adding a single semicolon would make the expression divergent.
e.g. The following would be a divergent if a semicolon were added after the `if` expression:
```rust
{ if panic!() { 0 } else { 1 } }
```
changelog: None
Lint `needless_borrow` and `explicit_auto_deref` on most union field accesses
Changes both lints to follow rustc's rules around auto-deref through `ManuallyDrop` union fields rather than just bailing on union fields.
changelog: [`needless_borrow`] & [`explicit_auto_deref`]: Lint on most union field accesses
[`map_identity`]: respect match ergonomics
Fixes#11764
Note: the original tests before this were slightly wrong themselves already and had to be changed. They were calling `map` on an iterator of `&(i32, i32)`s, so this PR would stop linting there, but they were meant to test something else unrelated to binding modes, so I just changed them to remove the references so that it iterates over owned values and they all bind by value. This way they continue to test what they checked for before: the identity function for tuple patterns.
changelog: [`map_identity`]: respect match ergonomics
Make SpanlessEq more consistent
1) Remove wildcard as requested in https://github.com/rust-lang/rust-clippy/issues/10267.
2) Implement `hir_utils::eq_expr` for `ExprKind::Closure`, `ExprKind::ConstBlock`, `ExprKind::InlineAsm` and `ExprKind::Yield`.
3) Reorder branches of `hir_utils::eq_expr` to be in alphabetical order.
---
changelog: none
Most notably, this commit changes the `pub use crate::*;` in that file
to `use crate::*;`. This requires a lot of `use` items in other crates
to be adjusted, because everything defined within `rustc_span::*` was
also available via `rustc_span::source_map::*`, which is bizarre.
The commit also removes `SourceMap::span_to_relative_line_string`, which
is unused.
Remove internal feature from clippy_utils
It's only used to gate a few `const`s, removing the feature gate means it doesn't have to be recompiled when moving between a normal and `-F internal` build/test/etc
changelog: none
Implement `gen` blocks in the 2024 edition
Coroutines tracking issue https://github.com/rust-lang/rust/issues/43122
`gen` block tracking issue https://github.com/rust-lang/rust/issues/117078
This PR implements `gen` blocks that implement `Iterator`. Most of the logic with `async` blocks is shared, and thus I renamed various types that were referring to `async` specifically.
An example usage of `gen` blocks is
```rust
fn foo() -> impl Iterator<Item = i32> {
gen {
yield 42;
for i in 5..18 {
if i.is_even() { continue }
yield i * 2;
}
}
}
```
The limitations (to be resolved) of the implementation are listed in the tracking issue
Add `waker_clone_and_wake` lint to check needless `Waker` clones
Check for patterns of `waker.clone().wake()` and replace them with `waker.wake_by_ref()`.
An alternative name could be `waker_clone_then_wake`
changelog: [ `waker_clone_wake`]: new lint
[`map_identity`]: allow closure with type annotations
Fixes#9122
`.map(|a: u32| a)` can help type inference, so we should probably allow this and not warn about "unnecessary map of the identity function"
changelog: [`map_identity`]: allow closure with type annotations
Deserialize `Msrv` directly in `Conf`
Gives the error a span pointing to the invalid config value
Also puts `Conf` itself in the `OnceLock` rather than just the `Msrv` for [the `register_late_mod_pass` work](https://github.com/rust-lang/rust/pull/116731) since it will be used from two different callbacks
changelog: none
side effect for `enum_variants`:
use .first() instead of .get(0) in enum_variants lint
move to_camel_case to str_util module
move module, enum and struct name repetitions check to a single file `item_name_repetitions`
rename enum_variants threshold config option
[`get_first`]: lint on non-primitive slices
Fixes#11594
I left the issue open for a couple days before making the PR to see if anyone has something to say, but it looks like there aren't any objections to removing this check that prevented linting on non-primitive slices, so here's the PR now.
There's a couple of instances in clippy itself where we now emit the lint. The actual relevant change is in the first commit and fixing the `.get(0)` instances in clippy itself is in the 2nd commit.
changelog: [`get_first`]: lint on non-primitive slices
Partially outline code inside the panic! macro
This outlines code inside the panic! macro in some cases. This is split out from https://github.com/rust-lang/rust/pull/115562 to exclude changes to rustc.
mir_to_const improvements
This simplifies some code and also fixes the float array handling to properly take into account the `offset`, and to work with little-endian targets.
Fixes https://github.com/rust-lang/rust-clippy/issues/11488
changelog: none
Don't store lazyness in `DefKind::TyAlias`
1. Don't store lazyness of a type alias in its `DefKind`, but instead via a query.
2. This allows us to treat type aliases as lazy if `#[feature(lazy_type_alias)]` *OR* if the alias contains a TAIT, rather than having checks for both in separate parts of the codebase.
r? `@oli-obk` cc `@fmease`
Improve invalid let expression handling
- Move all of the checks for valid let expression positions to parsing.
- Add a field to ExprKind::Let in AST/HIR to mark whether it's in a valid location.
- Suppress some later errors and MIR construction for invalid let expressions.
- Fix a (drop) scope issue that was also responsible for #104172.
Fixes#104172Fixes#104868
new lint: `iter_out_of_bounds`
Closes#11345
The original idea in the linked issue seemed to be just about arrays afaict, but I extended this to catch some other iterator sources such as `iter::once` or `iter::empty`.
I'm not entirely sure if this name makes a lot of sense now that it's not just about arrays anymore (specifically, not sure if you can call `.take(1)` on an `iter::Empty` to be "out of bounds"?).
changelog: [`iter_out_of_bounds`]: new lint
Update Clippy
r? `@oli-obk` Assigning you, because something broke with ui_test:
```
tests/ui/crashes/ice-7272.rs FAILED:
command: "<unknown>"
A bug in `ui_test` occurred: called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
full stderr:
```
(and that 103 times)
Thought I would ping you, before starting to investigate. Maybe you know what's going on.
[`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`
Closes#11371
This turned out to be a little more code than I originally thought, because the lint also makes sure to not lint if the user tries to mutate the option:
```rs
if option.is_some() {
option = None;
option.unwrap(); // don't lint here
}
```
... which means that even if we taught this lint to recognize `.as_mut()`, it would *still* not lint because that would count as a mutation. So we need to allow `.as_mut()` calls but reject other kinds of mutations.
Unfortunately it doesn't look like this is possible with `is_potentially_mutated` (seeing what kind of mutation happened).
This replaces it with a custom little visitor that does basically what it did before, but also allows `.as_mut()`.
changelog: [`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`
[`if_then_some_else_none`]: look into local initializers for early returns
Fixes#11394
As the PR title says, problem was that it only looked for early returns in semi statements. Local variables don't count as such, so it didn't count `let _v = x?;` (or even just `let _ = return;`) as a possible early return and didn't realize that it can't lint then.
Imo the `stmts_contains_early_return` function that was used before is redundant. `contains_return` could already do that if we just made the parameter a bit more generic, just like `for_each_expr`, which can already accept `&[Stmt]`
changelog: [`if_then_some_else_none`]: look into local initializers for early returns
Point at return type when it influences non-first `match` arm
When encountering code like
```rust
fn foo() -> i32 {
match 0 {
1 => return 0,
2 => "",
_ => 1,
}
}
```
Point at the return type and not at the prior arm, as that arm has type `!` which isn't influencing the arm corresponding to arm `2`.
Fix#78124.
Store the laziness of type aliases in their `DefKind`
Previously, we would treat paths referring to type aliases as *lazy* type aliases if the current crate had lazy type aliases enabled independently of whether the crate which the alias was defined in had the feature enabled or not.
With this PR, the laziness of a type alias depends on the crate it is defined in. This generally makes more sense to me especially if / once lazy type aliases become the default in a new edition and we need to think about *edition interoperability*:
Consider the hypothetical case where the dependency crate has an older edition (and thus eager type aliases), it exports a type alias with bounds & a where-clause (which are void but technically valid), the dependent crate has the latest edition (and thus lazy type aliases) and it uses that type alias. Arguably, the bounds should *not* be checked since at any time, the dependency crate should be allowed to change the bounds at will with a *non*-major version bump & without negatively affecting downstream crates.
As for the reverse case (dependency: lazy type aliases, dependent: eager type aliases), I guess it rules out anything from slight confusion to mild annoyance from upstream crate authors that would be caused by the compiler ignoring the bounds of their type aliases in downstream crates with older editions.
---
This fixes#114468 since before, my assumption that the type alias associated with a given weak projection was lazy (and therefore had its variances computed) did not necessarily hold in cross-crate scenarios (which [I kinda had a hunch about](https://github.com/rust-lang/rust/pull/114253#discussion_r1278608099)) as outlined above. Now it does hold.
`@rustbot` label F-lazy_type_alias
r? `@oli-obk`
Improve spans for indexing expressions
fixes#114388
Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location.
This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.
r? compiler-errors
Indexing is similar to method calls in having an arbitrary
left-hand-side and then something on the right, which is the main part
of the expression. Method calls already have a span for that right part,
but indexing does not. This means that long method chains that use
indexing have really bad spans, especially when the indexing panics and
that span in coverted into a panic location.
This does the same thing as method calls for the AST and HIR, storing an
extra span which is then put into the `fn_span` field in THIR.
Perform OpaqueCast field projection on HIR, too.
fixes#105819
This is necessary for closure captures in 2021 edition, as they capture individual fields, not the full mentioned variables. So it may try to capture a field of an opaque (because the hidden type is known to be something with a field).
See https://github.com/rust-lang/rust/pull/99806 for when and why we added OpaqueCast to MIR.
