Skip to content

Commit

Permalink
Allow to expect (adopted) (#15301)
Browse files Browse the repository at this point in the history
# Objective

> Rust 1.81 released the #[expect(...)] attribute, which works like
#[allow(...)] but throws a warning if the lint isn't raised. This is
preferred to #[allow(...)] because it tells us when it can be removed.

- Adopts the parts of #15118 that are complete, and updates the branch
so it can be merged.
- There were a few conflicts, let me know if I misjudged any of 'em.

Alice's
[recommendation](#15059 (comment))
seems well-taken, let's do this crate by crate now that @BD103 has done
the lion's share of this!

(Relates to, but doesn't yet completely finish #15059.)

Crates this _doesn't_ cover:

- bevy_input
- bevy_gilrs
- bevy_window
- bevy_winit
- bevy_state
- bevy_render
- bevy_picking
- bevy_core_pipeline
- bevy_sprite
- bevy_text
- bevy_pbr
- bevy_ui
- bevy_gltf
- bevy_gizmos
- bevy_dev_tools
- bevy_internal
- bevy_dylib

---------

Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
Co-authored-by: Ben Frankel <ben.frankel7@gmail.com>
Co-authored-by: Antony <antony.m.3012@gmail.com>
  • Loading branch information
4 people committed Sep 20, 2024
1 parent ebb57c5 commit fd329c0
Show file tree
Hide file tree
Showing 44 changed files with 182 additions and 100 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ keywords = ["game", "engine", "gamedev", "graphics", "bevy"]
license = "MIT OR Apache-2.0"
repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy"
rust-version = "1.80.0"
rust-version = "1.81.0"

[workspace]
exclude = [
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ pub(crate) enum AppError {
DuplicatePlugin { plugin_name: String },
}

#[allow(clippy::needless_doctest_main)]
/// [`App`] is the primary API for writing user applications. It automates the setup of a
/// [standard lifecycle](Main) and provides interface glue for [plugins](`Plugin`).
///
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_app/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ mod sealed {
where
$($plugins: Plugins<$param>),*
{
#[allow(non_snake_case, unused_variables)]
// We use `allow` instead of `expect` here because the lint is not generated for all cases.
#[allow(non_snake_case, reason = "`all_tuples!()` generates non-snake-case variable names.")]
#[allow(unused_variables, reason = "`app` is unused when implemented for the unit type `()`.")]
#[track_caller]
fn add_to_app(self, app: &mut App) {
let ($($plugins,)*) = self;
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_app/src/plugin_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,10 @@ impl PluginGroupBuilder {
/// Adds the plugin [`Plugin`] at the end of this [`PluginGroupBuilder`]. If the plugin was
/// already in the group, it is removed from its previous place.
// This is not confusing, clippy!
#[allow(clippy::should_implement_trait)]
#[expect(
clippy::should_implement_trait,
reason = "This does not emulate the `+` operator, but is more akin to pushing to a stack."
)]
pub fn add<T: Plugin>(mut self, plugin: T) -> Self {
let target_index = self.order.len();
self.order.push(TypeId::of::<T>());
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_asset/macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

use bevy_macro_utils::BevyManifest;
Expand Down
27 changes: 22 additions & 5 deletions crates/bevy_asset/src/io/embedded/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,13 @@ impl EmbeddedAssetRegistry {
/// running in a non-rust file). `asset_path` is the path that will be used to identify the asset in the `embedded`
/// [`AssetSource`]. `value` is the bytes that will be returned for the asset. This can be _either_ a `&'static [u8]`
/// or a [`Vec<u8>`].
#[allow(unused)]
#[cfg_attr(
not(feature = "embedded_watcher"),
expect(
unused_variables,
reason = "The `full_path` argument is not used when `embedded_watcher` is disabled."
)
)]
pub fn insert_asset(&self, full_path: PathBuf, asset_path: &Path, value: impl Into<Value>) {
#[cfg(feature = "embedded_watcher")]
self.root_paths
Expand All @@ -43,7 +49,13 @@ impl EmbeddedAssetRegistry {
/// running in a non-rust file). `asset_path` is the path that will be used to identify the asset in the `embedded`
/// [`AssetSource`]. `value` is the bytes that will be returned for the asset. This can be _either_ a `&'static [u8]`
/// or a [`Vec<u8>`].
#[allow(unused)]
#[cfg_attr(
not(feature = "embedded_watcher"),
expect(
unused_variables,
reason = "The `full_path` argument is not used when `embedded_watcher` is disabled."
)
)]
pub fn insert_meta(&self, full_path: &Path, asset_path: &Path, value: impl Into<Value>) {
#[cfg(feature = "embedded_watcher")]
self.root_paths
Expand All @@ -59,12 +71,17 @@ impl EmbeddedAssetRegistry {
self.dir.remove_asset(full_path)
}

/// Registers a `embedded` [`AssetSource`] that uses this [`EmbeddedAssetRegistry`].
// NOTE: unused_mut because embedded_watcher feature is the only mutable consumer of `let mut source`
#[allow(unused_mut)]
pub fn register_source(&self, sources: &mut AssetSourceBuilders) {
let dir = self.dir.clone();
let processed_dir = self.dir.clone();

#[cfg_attr(
not(feature = "embedded_watcher"),
expect(
unused_mut,
reason = "Variable is only mutated when `embedded_watcher` feature is enabled."
)
)]
let mut source = AssetSource::build()
.with_reader(move || Box::new(MemoryAssetReader { root: dir.clone() }))
.with_processed_reader(move || {
Expand Down
15 changes: 12 additions & 3 deletions crates/bevy_asset/src/io/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use thiserror::Error;

use super::{ErasedAssetReader, ErasedAssetWriter};

// Needed for doc strings.
#[allow(unused_imports)]
#[allow(unused_imports, reason = "Needed for documentation links.")]
use crate::io::{AssetReader, AssetWriter};

/// A reference to an "asset source", which maps to an [`AssetReader`] and/or [`AssetWriter`].
Expand Down Expand Up @@ -508,7 +507,17 @@ impl AssetSource {
/// `file_debounce_time` is the amount of time to wait (and debounce duplicate events) before returning an event.
/// Higher durations reduce duplicates but increase the amount of time before a change event is processed. If the
/// duration is set too low, some systems might surface events _before_ their filesystem has the changes.
#[allow(unused)]
#[cfg_attr(
any(
not(feature = "file_watcher"),
target_arch = "wasm32",
target_os = "android"
),
expect(
unused_variables,
reason = "The `path` and `file_debounce_wait_time` arguments are unused when on WASM, Android, or if the `file_watcher` feature is disabled."
)
)]
pub fn get_default_watcher(
path: String,
file_debounce_wait_time: Duration,
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@
//! This trait mirrors [`AssetLoader`] in structure, and works in tandem with [`AssetWriter`](io::AssetWriter), which mirrors [`AssetReader`](io::AssetReader).

// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![doc(
html_logo_url = "https://bevyengine.org/assets/icon.png",
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_asset/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ impl ErasedLoadedAsset {

/// Cast this loaded asset as the given type. If the type does not match,
/// the original type-erased asset is returned.
#[allow(clippy::result_large_err)]
#[expect(clippy::result_large_err, reason = "Function returns `Self` on error.")]
pub fn downcast<A: Asset>(mut self) -> Result<LoadedAsset<A>, ErasedLoadedAsset> {
match self.value.downcast::<A>() {
Ok(value) => Ok(LoadedAsset {
Expand Down
10 changes: 8 additions & 2 deletions crates/bevy_asset/src/processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ use std::{
use thiserror::Error;

// Needed for doc strings
#[allow(unused_imports)]
#[allow(unused_imports, reason = "Needed for documentation links.")]
use crate::io::{AssetReader, AssetWriter};

/// A "background" asset processor that reads asset values from a source [`AssetSource`] (which corresponds to an [`AssetReader`] / [`AssetWriter`] pair),
Expand Down Expand Up @@ -557,7 +557,13 @@ impl AssetProcessor {
/// This info will later be used to determine whether or not to re-process an asset
///
/// This will validate transactions and recover failed transactions when necessary.
#[allow(unused)]
#[cfg_attr(
any(target_arch = "wasm32", not(feature = "multi_threaded")),
expect(
dead_code,
reason = "This function is only used when the `multi_threaded` feature is enabled, and when not on WASM."
)
)]
async fn initialize(&self) -> Result<(), InitializeError> {
self.validate_transaction_log_and_recover().await;
let mut asset_infos = self.data.asset_infos.write().await;
Expand Down
17 changes: 12 additions & 5 deletions crates/bevy_asset/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,19 @@ impl ReflectAsset {
}

/// Equivalent of [`Assets::get_mut`]
#[allow(unsafe_code)]
pub fn get_mut<'w>(
&self,
world: &'w mut World,
handle: UntypedHandle,
) -> Option<&'w mut dyn Reflect> {
// SAFETY: unique world access
unsafe { (self.get_unchecked_mut)(world.as_unsafe_world_cell(), handle) }
#[expect(
unsafe_code,
reason = "Use of unsafe `Self::get_unchecked_mut()` function."
)]
unsafe {
(self.get_unchecked_mut)(world.as_unsafe_world_cell(), handle)
}
}

/// Equivalent of [`Assets::get_mut`], but works with an [`UnsafeWorldCell`].
Expand Down Expand Up @@ -83,7 +88,10 @@ impl ReflectAsset {
/// violating Rust's aliasing rules. To avoid this:
/// * Only call this method if you know that the [`UnsafeWorldCell`] may be used to access the corresponding `Assets<T>`
/// * Don't call this method more than once in the same scope.
#[allow(unsafe_code)]
#[expect(
unsafe_code,
reason = "This function calls unsafe code and has safety requirements."
)]
pub unsafe fn get_unchecked_mut<'w>(
&self,
world: UnsafeWorldCell<'w>,
Expand All @@ -108,7 +116,6 @@ impl ReflectAsset {
}

/// Equivalent of [`Assets::len`]
#[allow(clippy::len_without_is_empty)] // clippy expects the `is_empty` method to have the signature `(&self) -> bool`
pub fn len(&self, world: &World) -> usize {
(self.len)(world)
}
Expand Down Expand Up @@ -137,7 +144,7 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
get_unchecked_mut: |world, handle| {
// SAFETY: `get_unchecked_mut` must be called with `UnsafeWorldCell` having access to `Assets<A>`,
// and must ensure to only have at most one reference to it live at all times.
#[allow(unsafe_code)]
#[expect(unsafe_code, reason = "Uses `UnsafeWorldCell::get_resource_mut()`.")]
let assets = unsafe { world.get_resource_mut::<Assets<A>>().unwrap().into_inner() };
let asset = assets.get_mut(&handle.typed_debug_checked());
asset.map(|asset| asset as &mut dyn Reflect)
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_asset/src/server/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ impl AssetInfos {
.unwrap()
}

#[allow(clippy::too_many_arguments)]
#[expect(
clippy::too_many_arguments,
reason = "Arguments needed so that both `create_loading_handle_untyped()` and `get_or_create_path_handle_internal()` may share code."
)]
fn create_handle_internal(
infos: &mut HashMap<UntypedAssetId, AssetInfo>,
handle_providers: &TypeIdMap<AssetHandleProvider>,
Expand Down
4 changes: 1 addition & 3 deletions crates/bevy_asset/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ use std::{any::TypeId, path::Path, sync::Arc};
use std::{future::Future, panic::AssertUnwindSafe};
use thiserror::Error;

// Needed for doc string
#[allow(unused_imports)]
#[allow(unused_imports, reason = "Needed for documentation links.")]
use crate::io::{AssetReader, AssetWriter};

/// Loads and tracks the state of [`Asset`] values from a configured [`AssetReader`]. This can be used to kick off new asset loads and
Expand Down Expand Up @@ -1395,7 +1394,6 @@ pub fn handle_internal_asset_events(world: &mut World) {
}

/// Internal events for asset load results
#[allow(clippy::large_enum_variant)]
pub(crate) enum InternalAssetEvent {
Loaded {
id: UntypedAssetId,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![forbid(unsafe_code)]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![doc(
Expand Down
2 changes: 0 additions & 2 deletions crates/bevy_core_pipeline/src/skybox/prepass.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![warn(missing_docs)]

//! Adds motion vector support to skyboxes. See [`SkyboxPrepassPipeline`] for details.

use bevy_asset::Handle;
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_derive/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![forbid(unsafe_code)]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![doc(
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_diagnostic/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![forbid(unsafe_code)]
#![doc(
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

extern crate proc_macro;
Expand Down
3 changes: 0 additions & 3 deletions crates/bevy_ecs/macros/src/query_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,13 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
let read_only_struct_name = if attributes.is_mutable {
Ident::new(&format!("{struct_name}ReadOnly"), Span::call_site())
} else {
#[allow(clippy::redundant_clone)]
struct_name.clone()
};

let item_struct_name = Ident::new(&format!("{struct_name}Item"), Span::call_site());
let read_only_item_struct_name = if attributes.is_mutable {
Ident::new(&format!("{struct_name}ReadOnlyItem"), Span::call_site())
} else {
#[allow(clippy::redundant_clone)]
item_struct_name.clone()
};

Expand All @@ -122,7 +120,6 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream {
let new_ident = Ident::new(&format!("{struct_name}ReadOnlyFetch"), Span::call_site());
ensure_no_collision(new_ident, tokens.clone())
} else {
#[allow(clippy::redundant_clone)]
fetch_struct_name.clone()
};

Expand Down
10 changes: 8 additions & 2 deletions crates/bevy_ecs/macros/src/world_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ use proc_macro2::Ident;
use quote::quote;
use syn::{Attribute, Fields, ImplGenerics, TypeGenerics, Visibility, WhereClause};

#[allow(clippy::too_many_arguments)]
#[expect(
clippy::too_many_arguments,
reason = "Required to generate the entire item structure."
)]
pub(crate) fn item_struct(
path: &syn::Path,
fields: &Fields,
Expand Down Expand Up @@ -52,7 +55,10 @@ pub(crate) fn item_struct(
}
}

#[allow(clippy::too_many_arguments)]
#[expect(
clippy::too_many_arguments,
reason = "Required to generate the entire world query implementation."
)]
pub(crate) fn world_query_impl(
path: &syn::Path,
struct_name: &Ident,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/schedule/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,7 +1893,7 @@ impl ScheduleGraph {
&'a self,
ambiguities: &'a [(NodeId, NodeId, Vec<ComponentId>)],
components: &'a Components,
) -> impl Iterator<Item = (String, String, Vec<&str>)> + 'a {
) -> impl Iterator<Item = (String, String, Vec<&'a str>)> + 'a {
ambiguities
.iter()
.map(move |(system_a, system_b, conflicts)| {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_encase_derive/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![forbid(unsafe_code)]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![doc(
Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_log/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ use {bevy_ecs::system::Resource, bevy_utils::synccell::SyncCell};
/// Wrapper resource for `tracing-chrome`'s flush guard.
/// When the guard is dropped the chrome log is written to file.
#[cfg(feature = "tracing-chrome")]
#[allow(dead_code)]
#[expect(
dead_code,
reason = "`FlushGuard` never needs to be read, it just needs to be kept alive for the `App`'s lifetime."
)]
#[derive(Resource)]
pub(crate) struct FlushGuard(SyncCell<tracing_chrome::FlushGuard>);

Expand Down Expand Up @@ -187,7 +190,6 @@ impl Default for LogPlugin {
}

impl Plugin for LogPlugin {
#[cfg_attr(not(feature = "tracing-chrome"), allow(unused_variables))]
fn build(&self, app: &mut App) {
#[cfg(feature = "trace")]
{
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_mikktspace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
unsafe_op_in_unsafe_fn,
clippy::all,
clippy::undocumented_unsafe_blocks,
clippy::ptr_cast_constness
clippy::ptr_cast_constness,
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
missing_docs
)]
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![doc(
html_logo_url = "https://bevyengine.org/assets/icon.png",
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
// FIXME(15321): solve CI failures, then replace with `#![expect()]`.
#![allow(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![deny(unsafe_code)]
#![doc(
Expand Down
Loading

0 comments on commit fd329c0

Please sign in to comment.