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: &mut Commands function argument #127

Open
BD103 opened this issue Oct 7, 2024 · 1 comment
Open

Add lint: &mut Commands function argument #127

BD103 opened this issue Oct 7, 2024 · 1 comment
Labels
A-Linter Related to the linter and custom lints C-Feature Make something new possible

Comments

@BD103
Copy link
Member

BD103 commented Oct 7, 2024

Quoting @ItsDoot in bevyengine/bevy#15657:

How can Bevy's documentation be improved?

We should more clearly and directly recommend that developers write their helper functions to take Commands (and friends) directly, instead of passing as references (&Commands). And by doing so, these helpers can look just like systems. As an example:

fn my_system(commands: Commands, foo: Query<&Foo>) {
    // we should recommend this:
    good_helper(commands.reborrow(), foo.reborrow());
    // instead of this:
    bad_helper(&mut commands, &foo);
}

// This is kind of annoying:
fn bad_helper(commands: &mut Commands, foo: &Query<&Foo>) {
    // ...
}

// This is easier to deal with:
fn good_helper(commands: Commands, foo: Query<&Foo>) {
    // ...
}

We should recommend reborrowing in the top level docs for Commands and Query.

We can write a lint that would warn against taking &mut Commands in a function! It would probably be in the pedantic category, though.

@BD103 BD103 added A-Linter Related to the linter and custom lints C-Feature Make something new possible labels Oct 7, 2024
@janhohenheim
Copy link
Member

TIL about reborrow 👀

@BD103 BD103 added this to the `bevy_lint` 0.2.0 milestone Oct 16, 2024
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-Feature Make something new possible
Projects
Status: Todo
Development

No branches or pull requests

2 participants