bevy_lint: Add zst_query lint

bevy_lint/src/lib.rs

@@ -12,9 +12,11 @@
 
 // This is a list of every single `rustc` crate used within this library. If you need another, add
 // it here!
+extern crate rustc_abi;
 extern crate rustc_driver;
 extern crate rustc_errors;
 extern crate rustc_hir;
+extern crate rustc_hir_analysis;
 extern crate rustc_interface;
 extern crate rustc_lint;
 extern crate rustc_middle;
@@ -26,5 +28,6 @@ pub mod groups;
 mod lint;
 pub mod lints;
 mod paths;
+mod utils;
 
 pub use self::callback::BevyLintCallback;

bevy_lint/src/lints/mod.rs

@@ -10,6 +10,7 @@ pub mod main_return_without_appexit;
 pub mod missing_reflect;
 pub mod panicking_methods;
 pub mod plugin_not_ending_in_plugin;
+pub mod zst_query;
 
 pub(crate) static LINTS: &[&BevyLint] = &[
     insert_event_resource::INSERT_EVENT_RESOURCE,
@@ -18,6 +19,7 @@ pub(crate) static LINTS: &[&BevyLint] = &[
     missing_reflect::MISSING_REFLECT,
     panicking_methods::PANICKING_WORLD_METHODS,
     plugin_not_ending_in_plugin::PLUGIN_NOT_ENDING_IN_PLUGIN,
+    zst_query::ZST_QUERY,
 ];
 
 pub(crate) fn register_lints(store: &mut LintStore) {
@@ -31,4 +33,5 @@ pub(crate) fn register_passes(store: &mut LintStore) {
     store.register_late_pass(|_| Box::new(missing_reflect::MissingReflect));
     store.register_late_pass(|_| Box::new(panicking_methods::PanickingMethods));
     store.register_late_pass(|_| Box::new(plugin_not_ending_in_plugin::PluginNotEndingInPlugin));
+    store.register_late_pass(|_| Box::new(zst_query::ZstQuery));
 }

bevy_lint/src/lints/zst_query.rs

@@ -0,0 +1,145 @@
+//! Checks for queries that query for a zero-sized type.
+//!
+//! # Motivation
+//!
+//! Zero-sized types (ZSTs) are types that have no size as a result of containing no runtime data.
+//! In Bevy, such types are often used as marker components and are best used as filters.
+//!
+//! # Example
+//!
+//! ```
+//! # use bevy::prelude::*;
+//!
+//! #[derive(Component)]
+//! struct Player;
+//!
+//! fn move_player(mut query: Query<(&mut Transform, &Player)>) {
+//!     // ...
+//! }
+//! ```
+//!
+//! Use instead:
+//!
+//! ```
+//! # use bevy::prelude::*;
+//!
+//! #[derive(Component)]
+//! struct Player;
+//!
+//! fn move_player(query: Query<&mut Transform, With<Player>>) {
+//!     // ...
+//! }
+//! ```
+
+use crate::{
+    declare_bevy_lint,
+    utils::hir_parse::{detuple, generic_type_at},
+};
+use clippy_utils::{
+    diagnostics::span_lint_and_help,
+    ty::{is_normalizable, match_type},
+};
+use rustc_abi::Size;
+use rustc_hir_analysis::collect::ItemCtxt;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{
+    layout::{LayoutOf, TyAndLayout},
+    Ty,
+};
+use rustc_session::declare_lint_pass;
+
+declare_bevy_lint! {
+    pub ZST_QUERY,
+    RESTRICTION,
+    "query for a zero-sized type"
+}
+
+declare_lint_pass! {
+    ZstQuery => [ZST_QUERY.lint]
+}
+
+impl<'tcx> LateLintPass<'tcx> for ZstQuery {
+    fn check_ty(&mut self, cx: &LateContext<'tcx>, hir_ty: &'tcx rustc_hir::Ty<'tcx>) {
+        let item_cx = ItemCtxt::new(cx.tcx, hir_ty.hir_id.owner.def_id);
+        let ty = item_cx.lower_ty(hir_ty);
+
+        let Some(query_kind) = QueryKind::try_from_ty(cx, ty) else {
+            return;
+        };
+
+        let Some(query_data_ty) = generic_type_at(cx, hir_ty, 2) else {
+            return;
+        };
+
+        for hir_ty in detuple(*query_data_ty) {
+            let ty = item_cx.lower_ty(&hir_ty);
+
+            // We want to make sure we're evaluating `Foo` and not `&Foo`/`&mut Foo`
+            let peeled = ty.peel_refs();
+
+            if !is_zero_sized(cx, peeled).unwrap_or_default() {
+                continue;
+            }
+
+            // TODO: We can also special case `Option<&Foo>`/`Option<&mut Foo>` to
+            //       instead suggest `Has<Foo>`
+            span_lint_and_help(
+                cx,
+                ZST_QUERY.lint,
+                hir_ty.span,
+                ZST_QUERY.lint.desc,
+                None,
+                query_kind.help(&peeled),
+            );
+        }
+    }
+}
+
+enum QueryKind {
+    Query,
+}
+
+impl QueryKind {
+    fn try_from_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Self> {
+        if match_type(cx, ty, &crate::paths::QUERY) {
+            Some(Self::Query)
+        } else {
+            None
+        }
+    }
+
+    fn help(&self, ty: &Ty<'_>) -> String {
+        // It should be noted that `With<Foo>` is not always the best filter to suggest.
+        // While it's most often going to be what users want, there's also `Added<Foo>`
+        // and `Changed<Foo>` which might be more appropriate in some cases
+        // (i.e. users are calling `foo.is_added()` or `foo.is_changed()` in the body of
+        // the system).
+        // In the future, we might want to span the usage site of `is_added()`/`is_changed()`
+        // and suggest to use `Added<Foo>`/`Changed<Foo>` instead.
+        match self {
+            Self::Query => format!("consider using a filter instead: `With<{ty}>`"),
+        }
+    }
+}
+
+/// Checks if a type is zero-sized.
+///
+/// Returns:
+/// - `Some(true)` if the type is most likely a ZST
+/// - `Some(false)` if the type is most likely not a ZST
+/// - `None` if we cannot determine the size (e.g., type is not normalizable)
+fn is_zero_sized<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<bool> {
+    // `cx.layout_of()` panics if the type is not normalizable.
+    if !is_normalizable(cx, cx.param_env, ty) {
+        return None;
+    }
+
+    // Note: we don't use `approx_ty_size` from `clippy_utils` here
+    // because it will return `0` as the default value if the type is not
+    // normalizable, which will put us at risk of emitting more false positives.
+    if let Ok(TyAndLayout { layout, .. }) = cx.layout_of(ty) {
+        Some(layout.size() == Size::ZERO)
+    } else {
+        None
+    }
+}

bevy_lint/src/utils/hir_parse.rs

@@ -0,0 +1,44 @@
+//! Utility functions for parsing HIR types.
+
+use rustc_hir::{GenericArg, Node, QPath, Ty, TyKind};
+use rustc_lint::LateContext;
+
+/// Returns the list of types inside a tuple type.
+///
+/// If the type is not a tuple, returns a list containing the type itself.
+///
+/// This function will work for both tuples and references to tuples,
+/// such as `(f32, &str)` and `&(f32, &str)`.
+pub fn detuple(ty: Ty<'_>) -> Vec<Ty<'_>> {
+    if let TyKind::Tup(items) = ty.peel_refs().kind {
+        items.to_vec()
+    } else {
+        vec![ty]
+    }
+}
+
+/// Gets the [`Ty`] for a generic argument at the specified index.
+///
+/// If the generic argument is not a type, returns `None`.
+pub fn generic_type_at<'tcx>(
+    cx: &LateContext<'tcx>,
+    hir_ty: &'tcx Ty<'tcx>,
+    index: usize,
+) -> Option<&'tcx Ty<'tcx>> {
+    let generic_arg = generic_at(hir_ty, index)?;
+    let generic_node = cx.tcx.hir_node(generic_arg.hir_id());
+
+    if let Node::Ty(ty) = generic_node {
+        Some(ty)
+    } else {
+        None
+    }
+}
+/// Returns the [`GenericArg`] at the given index.
+pub fn generic_at<'hir>(hir_ty: &'hir Ty<'hir>, index: usize) -> Option<&'hir GenericArg<'hir>> {
+    let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind else {
+        return None;
+    };
+
+    path.segments.last()?.args().args.get(index)
+}

bevy_lint/src/utils/mod.rs

@@ -0,0 +1 @@
+pub mod hir_parse;

bevy_lint/tests/ui/zst_query/query.rs

@@ -0,0 +1,89 @@
+//! This tests the `zst_query` lint, specifically when triggered on the `Query` type.
+
+#![feature(register_tool)]
+#![register_tool(bevy)]
+#![deny(bevy::zst_query)]
+
+use bevy::prelude::*;
+use std::marker::PhantomData;
+
+#[derive(Component)]
+struct ZST;
+
+#[derive(Component)]
+#[allow(dead_code)]
+struct NonZST(u32);
+
+#[derive(Component)]
+#[allow(dead_code)]
+struct Generic<T: Sized + Send + Sync + 'static>(T);
+
+#[derive(Component)]
+#[allow(dead_code)]
+struct Phantom<T>(PhantomData<T>);
+
+fn main() {
+    App::new()
+        .add_systems(
+            Startup,
+            (
+                unit_query,
+                immutable_zst,
+                mutable_zst,
+                immutable_zst_tuple,
+                mutable_zst_tuple,
+                immutable_query,
+                mutable_query,
+                generic_immutable_query::<u32>,
+                generic_immutable_zst,
+                generic_mutable_zst,
+                generic_mutable_query::<u32>,
+                immutable_query_tuple,
+                mutable_query_tuple,
+                phantom_data_query,
+            ),
+        )
+        .run();
+}
+
+fn unit_query(_query: Query<()>) {}
+
+//~| HELP: consider using a filter instead: `With<ZST>`
+//~v ERROR: query for a zero-sized type
+fn immutable_zst(_query: Query<&ZST>) {}
+
+//~| HELP: consider using a filter instead: `With<ZST>`
+//~v ERROR: query for a zero-sized type
+fn mutable_zst(_query: Query<&mut ZST>) {}
+
+//~| HELP: consider using a filter instead: `With<ZST>`
+//~v ERROR: query for a zero-sized type
+fn immutable_zst_tuple(_query: Query<(Entity, &ZST)>) {}
+
+//~| HELP: consider using a filter instead: `With<ZST>`
+//~v ERROR: query for a zero-sized type
+fn mutable_zst_tuple(_query: Query<(Entity, &mut ZST)>) {}
+
+fn immutable_query(_query: Query<&NonZST>) {}
+
+fn mutable_query(_query: Query<&mut NonZST>) {}
+
+fn generic_immutable_query<T: Sized + Send + Sync + 'static>(_query: Query<&Generic<T>>) {}
+
+fn generic_mutable_query<T: Sized + Send + Sync + 'static>(_query: Query<&mut Generic<T>>) {}
+
+//~| HELP: consider using a filter instead: `With<Generic<ZST>>`
+//~v ERROR: query for a zero-sized type
+fn generic_immutable_zst(_query: Query<&Generic<ZST>>) {}
+
+//~| HELP: consider using a filter instead: `With<Generic<ZST>>`
+//~v ERROR: query for a zero-sized type
+fn generic_mutable_zst(_query: Query<&mut Generic<ZST>>) {}
+
+fn immutable_query_tuple(_query: Query<(Entity, &NonZST)>) {}
+
+fn mutable_query_tuple(_query: Query<(Entity, &mut NonZST)>) {}
+
+//~| HELP: consider using a filter instead: `With<Phantom<NonZST>>`
+//~v ERROR: query for a zero-sized type
+fn phantom_data_query(_query: Query<&Phantom<NonZST>>) {}

bevy_lint/tests/ui/zst_query/query.stderr

@@ -0,0 +1,63 @@
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:53:32
+   |
+53 | fn immutable_zst(_query: Query<&ZST>) {}
+   |                                ^^^^
+   |
+   = help: consider using a filter instead: `With<ZST>`
+note: the lint level is defined here
+  --> tests/ui/zst_query/query.rs:5:9
+   |
+5  | #![deny(bevy::zst_query)]
+   |         ^^^^^^^^^^^^^^^
+
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:57:30
+   |
+57 | fn mutable_zst(_query: Query<&mut ZST>) {}
+   |                              ^^^^^^^^
+   |
+   = help: consider using a filter instead: `With<ZST>`
+
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:61:47
+   |
+61 | fn immutable_zst_tuple(_query: Query<(Entity, &ZST)>) {}
+   |                                               ^^^^
+   |
+   = help: consider using a filter instead: `With<ZST>`
+
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:65:45
+   |
+65 | fn mutable_zst_tuple(_query: Query<(Entity, &mut ZST)>) {}
+   |                                             ^^^^^^^^
+   |
+   = help: consider using a filter instead: `With<ZST>`
+
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:77:40
+   |
+77 | fn generic_immutable_zst(_query: Query<&Generic<ZST>>) {}
+   |                                        ^^^^^^^^^^^^^
+   |
+   = help: consider using a filter instead: `With<Generic<ZST>>`
+
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:81:38
+   |
+81 | fn generic_mutable_zst(_query: Query<&mut Generic<ZST>>) {}
+   |                                      ^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using a filter instead: `With<Generic<ZST>>`
+
+error: query for a zero-sized type
+  --> tests/ui/zst_query/query.rs:89:37
+   |
+89 | fn phantom_data_query(_query: Query<&Phantom<NonZST>>) {}
+   |                                     ^^^^^^^^^^^^^^^^
+   |
+   = help: consider using a filter instead: `With<Phantom<NonZST>>`
+
+error: aborting due to 7 previous errors
+