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: unneeded &mut self methods for Query #107

Open
akimakinai opened this issue Sep 25, 2024 · 3 comments
Open

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

akimakinai opened this issue Sep 25, 2024 · 3 comments
Labels
A-Linter Related to the linter and custom lints C-Feature Make something new possible

Comments

@akimakinai
Copy link

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
Copy link
Member

BD103 commented Sep 25, 2024

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

@BD103 BD103 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2024
@akimakinai
Copy link
Author

akimakinai commented Sep 25, 2024

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
Copy link
Member

BD103 commented Sep 25, 2024

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.

@BD103 BD103 reopened this Sep 25, 2024
@BD103 BD103 added A-Linter Related to the linter and custom lints C-Feature Make something new possible labels Sep 25, 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