Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint: panicking_query_methods #95

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

BD103
Copy link
Member

@BD103 BD103 commented Sep 20, 2024

As per @alice-i-cecile's request! Part of #58.

This adds the bevy::panicking_query_methods lint, which searches for usage of panicking methods like Query::single(). It is currently Allow-by-default, so you need to explicitly opt-in to use it. (In the project it's under the Restriction category.)

This lint checks both Query and QueryState. I will write a lint for resources in another PR. :)

Testing

Wow, I'm so glad you asked! (You asked how to test this lint, right?)

It's a bit trickier than previous lints because it is off by default. 'Tis no matter: simply switch Allow to Warn locally:

declare_tool_lint! {
    pub bevy::PANICKING_QUERY_METHODS,
-    Allow,
+    Warn,
    "called a `Query` or `QueryState` method that can panic when a non-panicking alternative exists"
}

I used the following example when testing. Paste it into bevy_lint/examples/lint_test.rs, run cd bevy_lint, then run cargo build && cargo run -- --example lint_test.

use bevy::prelude::*;

#[derive(Component)]
struct MyComponent;

fn main() {
    App::new().add_systems(Update, my_system);

    let mut world = World::new();
    let mut query_state = QueryState::<&mut MyComponent>::new(&mut world);

    let _ = query_state.single_mut(&mut world);
}

fn my_system(query: Query<&MyComponent>) {
    let _ = query.many([]);
}

The warnings should look like this:

warning: called a `QueryState` method that can panic when a non-panicking alternative exists
  --> bevy_lint/examples/lint_test.rs:12:25
   |
12 |     let _ = query_state.single_mut(&mut world);
   |                         ^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: use `query_state.get_single_mut(&mut world)` and handle the `Result`
   = note: `#[warn(bevy::panicking_query_methods)]` on by default

warning: called a `Query` method that can panic when a non-panicking alternative exists
  --> bevy_lint/examples/lint_test.rs:16:19
   |
16 |     let _ = query.many([]);
   |                   ^^^^^^^^
   |
   = help: use `query.get_many([])` and handle the `Result`

warning: `bevy_lint` (example "lint_test") generated 2 warnings

Fiddle around with it and try to break it! Good luck, though. Unless you use #94, this is the most foolproof lint I've written so far! >:D

@BD103 BD103 added A-Linter Related to the linter and custom lints C-Enhancement A general improvement labels Sep 20, 2024
@janhohenheim
Copy link
Member

There's a little list of panicking APIs in bevyengine/bevy#14275 FYI :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Related to the linter and custom lints C-Enhancement A general improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants