TheBevyFlock / bevy_cli

A Bevy CLI tool.
Apache License 2.0
19 stars 4 forks source link

Add lint: unneeded `&mut self` methods for Query #107

Open akimakinai opened 3 days ago

akimakinai commented 3 days ago

The following code compiles without warning:

fn foo(mut q: Query<&C>) { // No mutable query
    for c in q.iter_mut() {
        // c: &C
    }
}

however, it should be written as:

fn foo(q: Query<&C>) {
    for c in q.iter() {
    }
}

This also applies to cases like for ... in &mut q, iter_combinations_mut, get_mut, etc. The linter should suggest using &self counterparts and ideally remove unnecessary mut from the variable.

BD103 commented 3 days ago

Thanks for the suggestion! This was previously suggested in #37, so I'm going to close this as a duplicate.

akimakinai commented 3 days ago

I thought Query<&mut C> whose uses are immutable (#37) and Query<&C> whose uses are redundantly mutable (this) would be separate lints (with largely different implementations/messages/suggestions), but probably issues can be merged.

Edit: Does #37 intend to just check whether e.g. iter_mut() is called on a Query containing &mut C, rather than tracking individual values are actually mutated? Sorry, implementations would be mostly same in that case.

BD103 commented 3 days ago

I thought Query<&mut C> whose uses are immutable (#37) and Query<&C> whose uses are redundantly mutable (this) would be separate lints (with largely different implementations/messages/suggestions), but probably issues can be merged.

Good point, I'll keep this separate then.