Update linter to toolchain nightly-2025-05-14
#456
Conversation
BD103
added
A-Linter
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
`find_crates()` now returns a slice, since it caches the results. This means it is espensive to call the function the first time, but not nearly so when executed again. Since we just care about the length of the slice, nothing changes for us.
`is_normalizable()` was originally used because `layout_of()` would panic if the type wasn't normalizable. `layout_of()` was later fixed, so the check isn't necessary anymore. See rust-lang/rust-clippy#14717 for details.
It was removed in rust-lang/rust-clippy#14650. We can either call `Symbol::intern()` manually (which is what this commit does), or use the new preinterned symbol system (https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/interface/struct.Config.html#structfield.extra_symbols).
It used to be re-exported in `rustc_middle::ty`, but no longer is.
It's no longer used, now that we pre-intern symbols.
…prove docs Turns out the compiler will ICE if there are multiple extra symbols with the same value. Additionally, `clippy_utils` depends on its extra symbols starting directly after `PREDEFINED_SYMBOLS_COUNT`, so we need to include it first.
The method got renamed, it took me _way_ too long to figure that out. I ended up removing it entirely in `borrowed_reborrowable` since we already have access to the argument names through the function body. (This was before I realized `fn_arg_names()` wasn't just outright deleted.) rust-lang/rust#139510
These were added after the PR was initially created.
This is a little `cargo update` that makes sure our dependencies are
up-to-date! There are a few dependencies that have newer versions that
we cannot update to yet:
```sh
$ cargo update -v
Updating crates.io index
Locking 0 packages to latest Rust 1.88.0-nightly compatible versions
Unchanged clippy_utils v0.1.88 (available: v0.1.89)
Unchanged matchit v0.8.4 (available: v0.8.6)
Unchanged rhai v1.21.0 (available: v1.22.2)
Unchanged tempfile v3.19.1 (available: v3.20.0)
```
- `clippy_utils` will be updated #456
- `matchit` is stuck due to `axum`'s dependency on it, nothing we can do
there
- `rhai` and `tempfile` are stuck due to `cargo-generate`'s dependency
on them, also nothing to do there
There was a problem hiding this comment.
CI will fail testing this action because it's trying to install the linter from the main branch, not rust-2025-05-14. This is a shortcoming of CI, and can be safely ignored!
| // Used to access the index of repeating macro input in `declare_bevy_symbols!`. | ||
| #![feature(macro_metavar_expr)] |
There was a problem hiding this comment.
The RFC for this feature is really interesting! It lets us do this kind of thing:
#[macro_export]
macro_rules! vec {
( $( $x:expr ),* ) => {
{
// `${count(x)}` counts the amount of times `$x` is specified, and evaluates to that number!
// We can't do this in stable Rust without proc-macros, which can be quite slow.
let mut temp_vec = Vec::with_capacity(${count(x)});
$(
temp_vec.push($x);
)*
temp_vec
}
};
}In the linter I use ${index()} in declare_bevy_symbols!. Check out that for more details.
| $( | ||
| @default = { | ||
| $($default_field:ident: $default_ty:ty = $default_value:expr),* $(,)? | ||
| }, | ||
| )? |
There was a problem hiding this comment.
We no longer need @default, as its primary purpose was to intern symbols we used. We now have the sym module for that!
| impl ::std::default::Default for $name { | ||
| fn default() -> Self { | ||
| Self { | ||
| $($($default_field: $default_value),*)? | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
We no longer implement Default for lint passes, as they are unit structs with no fields.
There was a problem hiding this comment.
The underlying concept of pre-interning symbols is this:
We assign each unique string a unique number. The easiest way to do this is to count upward, meaning the first 50 strings use the numbers 0 through 49. Some numbers are already used by the Rust compiler, however, so Bevy's symbols cannot start at 0. Instead, they start at SYMBOL_OFFSET.
There is some added complexity because we need to intern clippy_utils's symbols as well, but I talk about it well in the comments. :)
| /// assert_eq!(extract_value!(Name), "Name"); | ||
| /// assert_eq!(extract_value!(Name: "value"), "value"); | ||
| /// ``` | ||
| macro_rules! extract_value { |
There was a problem hiding this comment.
This macro is purely meant to be used internally by declare_bevy_symbols!.
| add_systems, | ||
| app, | ||
| App, |
There was a problem hiding this comment.
This will result in the constants:
pub const add_systems: Symbol = Symbol::new(SYMBOL_OFFSET + 0);
pub const app: Symbol = Symbol::new(SYMBOL_OFFSET + 1);
pub const App: Symbol = Symbol::new(SYMBOL_OFFSET + 2);
// ...| > **Important** | ||
| > | ||
| > `bevy_app::app` is a [private module], but we still have to refer to it by name because [`struct App`] is within `bevy_app/src/app.rs`. Do not be tricked by re-exported types, such as `bevy::prelude::App`! | ||
| > | ||
| > [private module]: https://docs.rs/bevy_app/0.16.0-rc.2/src/bevy_app/lib.rs.html#26 | ||
| > [`struct App`]: https://docs.rs/bevy_app/0.16.0-rc.2/src/bevy_app/app.rs.html#78-88 |
There was a problem hiding this comment.
I'm going to move this advice to a separate article about the paths module, but that's for a follow-up PR.
| tcx: TyCtxt<'tcx>, | ||
| trait_path: &[&str], | ||
| ) -> impl Iterator<Item = Self> + use<'tcx> { | ||
| fn from_local_crate<'tcx, 'a>( |
There was a problem hiding this comment.
I think this is totally fine, we could remove the extra lifetime if we just return a Vec, but this would result in a way worse API imo.
Yeah, I'm not used to it either yet, but I've been following Clippy's and |
github-project-automation
bot
moved this from In Progress
to Done!
in BD103 Work Planning
Closes #506, follow up to #456. This is a relatively simple upgrade, only two changes affected us: - `ItemKind::Struct` and `ItemKind::Enum`'s fields were reordered. - `GenericArg::unpack()` was renamed to `GenericArg::kind()`. With this, we're all caught up to Rust's language latest beta language features!
Wow, Rust 1.88.0 just released, meaning we're two releases behind and need to update. This is the first of the upgrade PRs, which migrates us to
nightly-2025-05-14andclippy_utilsv0.1.89.Summary
Symbols are now pre-interned and available as constants in thesymmodule.@defaultsyntax indeclare_bevy_lint_pass!.LintPass::default()when registering passes.PathLookup.How to Review
Unfortunately this PR isn't the easiest to review commit-by-commit. I'd recommend looking at the main diff and reading over my annotations, as that may help clarify the changes. Let me know if you have any questions!