Skip to content

Commit

Permalink
Address code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MrGVSV committed Nov 10, 2024
1 parent 7463962 commit e4a80fa
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 48 deletions.
14 changes: 9 additions & 5 deletions bevy_lint/src/lints/zst_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,17 @@ impl QueryKind {
/// - `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;
}

let Ok(TyAndLayout { layout, .. }) = cx.layout_of(ty) else {
return None;
};

Some(layout.size() == Size::ZERO)
// 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
}
}
12 changes: 6 additions & 6 deletions bevy_lint/src/utils/hir_parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ 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.
pub(crate) fn detuple(ty: Ty<'_>) -> Vec<Ty<'_>> {
///
/// 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 {
Expand All @@ -17,7 +20,7 @@ pub(crate) fn detuple(ty: Ty<'_>) -> Vec<Ty<'_>> {
/// Gets the [`Ty`] for a generic argument at the specified index.
///
/// If the generic argument is not a type, returns `None`.
pub(crate) fn generic_type_at<'tcx>(
pub fn generic_type_at<'tcx>(
cx: &LateContext<'tcx>,
hir_ty: &'tcx Ty<'tcx>,
index: usize,
Expand All @@ -32,10 +35,7 @@ pub(crate) fn generic_type_at<'tcx>(
}
}
/// Returns the [`GenericArg`] at the given index.
pub(crate) fn generic_at<'hir>(
hir_ty: &'hir Ty<'hir>,
index: usize,
) -> Option<&'hir GenericArg<'hir>> {
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;
};
Expand Down
2 changes: 1 addition & 1 deletion bevy_lint/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
pub(crate) mod hir_parse;
pub mod hir_parse;
48 changes: 29 additions & 19 deletions bevy_lint/tests/ui/zst_query/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ use bevy::prelude::*;
use std::marker::PhantomData;

#[derive(Component)]
struct Foo;
struct ZST;

#[derive(Component)]
#[allow(dead_code)]
struct Bar(u32);
struct NonZST(u32);

#[derive(Component)]
#[allow(dead_code)]
struct Baz<T: Sized + Send + Sync + 'static>(T);
struct Generic<T: Sized + Send + Sync + 'static>(T);

#[derive(Component)]
#[allow(dead_code)]
Expand All @@ -35,6 +35,8 @@ fn main() {
immutable_query,
mutable_query,
generic_immutable_query::<u32>,
generic_immutable_zst,
generic_mutable_zst,
generic_mutable_query::<u32>,
immutable_query_tuple,
mutable_query_tuple,
Expand All @@ -46,34 +48,42 @@ fn main() {

fn unit_query(_query: Query<()>) {}

//~| HELP: consider using a filter instead: `With<Foo>`
//~| HELP: consider using a filter instead: `With<ZST>`
//~v ERROR: query for a zero-sized type
fn immutable_zst(_query: Query<&Foo>) {}
fn immutable_zst(_query: Query<&ZST>) {}

//~| HELP: consider using a filter instead: `With<Foo>`
//~| HELP: consider using a filter instead: `With<ZST>`
//~v ERROR: query for a zero-sized type
fn mutable_zst(_query: Query<&mut Foo>) {}
fn mutable_zst(_query: Query<&mut ZST>) {}

//~| HELP: consider using a filter instead: `With<Foo>`
//~| HELP: consider using a filter instead: `With<ZST>`
//~v ERROR: query for a zero-sized type
fn immutable_zst_tuple(_query: Query<(Entity, &Foo)>) {}
fn immutable_zst_tuple(_query: Query<(Entity, &ZST)>) {}

//~| HELP: consider using a filter instead: `With<Foo>`
//~| HELP: consider using a filter instead: `With<ZST>`
//~v ERROR: query for a zero-sized type
fn mutable_zst_tuple(_query: Query<(Entity, &mut Foo)>) {}
fn mutable_zst_tuple(_query: Query<(Entity, &mut ZST)>) {}

fn immutable_query(_query: Query<&Bar>) {}
fn immutable_query(_query: Query<&NonZST>) {}

fn mutable_query(_query: Query<&mut Bar>) {}
fn mutable_query(_query: Query<&mut NonZST>) {}

fn generic_immutable_query<T: Sized + Send + Sync + 'static>(_query: Query<&Baz<T>>) {}
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 Baz<T>>) {}
fn generic_mutable_query<T: Sized + Send + Sync + 'static>(_query: Query<&mut Generic<T>>) {}

fn immutable_query_tuple(_query: Query<(Entity, &Bar)>) {}
//~| 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 Bar)>) {}
fn mutable_query_tuple(_query: Query<(Entity, &mut NonZST)>) {}

//~| HELP: consider using a filter instead: `With<Phantom<Bar>>`
//~| HELP: consider using a filter instead: `With<Phantom<NonZST>>`
//~v ERROR: query for a zero-sized type
fn phantom_data_query(_query: Query<&Phantom<Bar>>) {}
fn phantom_data_query(_query: Query<&Phantom<NonZST>>) {}
50 changes: 33 additions & 17 deletions bevy_lint/tests/ui/zst_query/query.stderr
Original file line number Diff line number Diff line change
@@ -1,47 +1,63 @@
error: query for a zero-sized type
--> tests/ui/zst_query/query.rs:51:32
--> tests/ui/zst_query/query.rs:53:32
|
51 | fn immutable_zst(_query: Query<&Foo>) {}
53 | fn immutable_zst(_query: Query<&ZST>) {}
| ^^^^
|
= help: consider using a filter instead: `With<Foo>`
= 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:55:30
--> tests/ui/zst_query/query.rs:57:30
|
55 | fn mutable_zst(_query: Query<&mut Foo>) {}
57 | fn mutable_zst(_query: Query<&mut ZST>) {}
| ^^^^^^^^
|
= help: consider using a filter instead: `With<Foo>`
= help: consider using a filter instead: `With<ZST>`

error: query for a zero-sized type
--> tests/ui/zst_query/query.rs:59:47
--> tests/ui/zst_query/query.rs:61:47
|
59 | fn immutable_zst_tuple(_query: Query<(Entity, &Foo)>) {}
61 | fn immutable_zst_tuple(_query: Query<(Entity, &ZST)>) {}
| ^^^^
|
= help: consider using a filter instead: `With<Foo>`
= help: consider using a filter instead: `With<ZST>`

error: query for a zero-sized type
--> tests/ui/zst_query/query.rs:63:45
--> tests/ui/zst_query/query.rs:65:45
|
63 | fn mutable_zst_tuple(_query: Query<(Entity, &mut Foo)>) {}
65 | fn mutable_zst_tuple(_query: Query<(Entity, &mut ZST)>) {}
| ^^^^^^^^
|
= help: consider using a filter instead: `With<Foo>`
= help: consider using a filter instead: `With<ZST>`

error: query for a zero-sized type
--> tests/ui/zst_query/query.rs:79:37
--> tests/ui/zst_query/query.rs:77:40
|
79 | fn phantom_data_query(_query: Query<&Phantom<Bar>>) {}
| ^^^^^^^^^^^^^
77 | fn generic_immutable_zst(_query: Query<&Generic<ZST>>) {}
| ^^^^^^^^^^^^^
|
= help: consider using a filter instead: `With<Phantom<Bar>>`
= help: consider using a filter instead: `With<Generic<ZST>>`

error: aborting due to 5 previous errors
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

0 comments on commit e4a80fa

Please sign in to comment.