Skip to content

Add help in case ref and mut are switched #43451

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
wants to merge 1 commit into from

Conversation

GuillaumeGomez
Copy link
Member

Fixes #43286.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2017
@arielb1
Copy link
Contributor

arielb1 commented Jul 24, 2017

r? @petrochenkov

self.span_err(self.span, &format!("expected identifier, found {}",
self.this_token_descr()));
let desc = self.this_token_descr();
if desc == "keyword `ref`" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the correct way of doing this is self.token.is_keyword(keyword::Ref)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

self.this_token_descr()));
let desc = self.this_token_descr();
if desc == "keyword `ref`" {
self.span_err_help(self.span,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a prime candidate for span_suggestion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered for a while between the both of them. But if you prefer suggestion, I'll totally update to it.

@GuillaumeGomez
Copy link
Member Author

Updated.

@petrochenkov
Copy link
Contributor

fn parse_ident is not a good place for this diagnostics, it will fire in many totally unrelated situations.
Also, no recovery is happening.

I believe the correct fix should look like this:

--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -3523,8 +3523,18 @@ impl<'a> Parser<'a> {
             }
             // At this point, token != _, &, &&, (, [
             _ => if self.eat_keyword(keywords::Mut) {
-                // Parse mut ident @ pat
-                pat = self.parse_pat_ident(BindingMode::ByValue(Mutability::Mutable))?;
+                // Parse mut ident @ pat / mut ref ident @ pat
+                let mutref_span = self.prev_span.to(self.span);
+                let binding_mode = if self.eat_keyword(keywords::Ref) {
+                    self.diagnostic()
+                        .struct_span_err(mutref_span, "the order of `mut` and `ref` is incorrect")
+                        .span_suggestion(mutref_span, "try switching the order", "ref mut".into())
+                        .emit();
+                    BindingMode::ByRef(Mutability::Mutable)
+                } else {
+                    BindingMode::ByValue(Mutability::Mutable)
+                };
+                pat = self.parse_pat_ident(binding_mode)?;
             } else if self.eat_keyword(keywords::Ref) {
                 // Parse ref ident @ pat / ref mut ident @ pat
                 let mutbl = self.parse_mutability();

@GuillaumeGomez
Copy link
Member Author

@petrochenkov: Well, you did it. I can take your patch and apply but you sure you don't want to open a PR yourself? I'd feel bad to take it from you. :-/

@petrochenkov
Copy link
Contributor

@GuillaumeGomez
Okay, submitted #43489

@GuillaumeGomez GuillaumeGomez deleted the ref-mut-help branch July 26, 2017 18:09
bors added a commit that referenced this pull request Jul 27, 2017
Better diagnostics and recovery for `mut ref` in patterns

Fixes #43286
Supersedes #43451

r? @GuillaumeGomez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants