Skip to content

Improve usability for "make variable mut" suggestion #12618

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

Closed
camelid opened this issue Jun 22, 2022 · 7 comments
Closed

Improve usability for "make variable mut" suggestion #12618

camelid opened this issue Jun 22, 2022 · 7 comments
Labels
A-assists A-diagnostics diagnostics / error reporting C-feature Category: feature request

Comments

@camelid
Copy link
Member

camelid commented Jun 22, 2022

With code like

let v = vec![];
// ....
v.push(123);

RA suggests that I make v mut (which I expect!). However, in order to apply the suggestion, I have to navigate to the space between let and v, at which point it's no faster than typing it myself.

When I used to use RLS, I could press Cmd-. at the point of use (v.push) and have it apply the suggestion to the declaration place. Could RA do this as well? It'd be a great quality of life improvement IMO.

Originally posted on Zulip.

@flodiebold
Copy link
Member

Related #8449, though this one would be just about providing the rustc suggestion in more places. I think we used to do this, but changed it because it led to some problems?

@flodiebold flodiebold added A-assists C-feature Category: feature request labels Jun 22, 2022
@jonas-schievink
Copy link
Contributor

The easy solution here is to also provide the quick fix where the main error is reported. The problem is that, for other diagnostics, this will end up with quick fixes that say something like "consider removing this semicolon", while not pointing at a semicolon at all (selecting the quick fix will remove a semicolon somewhere else). This is very confusing and something I'd like to avoid.

Maybe this specific diagnostic could be implemented natively now? That way we'd have full control over how it looks.

@jonas-schievink jonas-schievink added the A-diagnostics diagnostics / error reporting label Jun 23, 2022
@camelid
Copy link
Member Author

camelid commented Jun 23, 2022

Maybe this specific diagnostic could be implemented natively now? That way we'd have full control over how it looks.

Hmm, I guess this particular error doesn't really need the borrow-checker?

@jonas-schievink
Copy link
Contributor

Seems to me that the only tricky part of the diagnostic is implicit borrows, so it has to check not only for &mut <X>, but also for mutable auto-ref in method calls, and also for mutable auto-deref (anything else?).

@camelid
Copy link
Member Author

camelid commented Jun 23, 2022

Yeah, I think as long as RA can see the type signature of the called function (including if it's in a trait), it should be fairly straightforward to determine if there's mutation (famous last words...).

@flodiebold
Copy link
Member

flodiebold commented Jun 23, 2022

We already keep track of autorefs / derefs. I think there's a bit of a tricky part with keeping track where we "need mut" during inference so that everything in &mut foo.bar[baz].qux gets marked as mutable; especially with Index vs. IndexMut and Deref vs. DerefMut. I don't think it's super complicated, but it takes some care and we have to figure out how to represent this information during inference (probably by looking into how rustc does it).

@HKalbasi
Copy link
Member

HKalbasi commented Jun 4, 2023

Native diagnostic is implemented in #14232. The suggestion for rustc diagnostic still only works on declaration but I think the native diagnostic is good enough that fixing rustc suggestion doesn't worth the effort.

@HKalbasi HKalbasi closed this as completed Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists A-diagnostics diagnostics / error reporting C-feature Category: feature request
Projects
None yet
Development

No branches or pull requests

4 participants