Skip to content

Commit

Permalink
in which ! is suggested for erroneous identifier not
Browse files Browse the repository at this point in the history
Impressing confused Python users with magical diagnostics is perhaps
worth this not-grossly-unreasonable (only 40ish lines) extra complexity
in the parser?

Thanks to Vadim Petrochenkov for guidance.

This resolves #46836.
  • Loading branch information
zackmdavis committed Apr 9, 2018
1 parent 944c401 commit ba0dd8e
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 2 deletions.
43 changes: 42 additions & 1 deletion src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2830,7 +2830,48 @@ impl<'a> Parser<'a> {
let (span, e) = self.interpolated_or_expr_span(e)?;
(lo.to(span), ExprKind::Box(e))
}
_ => return self.parse_dot_or_call_expr(Some(attrs))
token::Ident(..) if self.token.is_ident_named("not") => {
// `not` is just an ordinary identifier in Rust-the-language,
// but as `rustc`-the-compiler, we can issue clever diagnostics
// for confused users who really want to say `!`
let token_cannot_continue_expr = |t: &token::Token| match *t {
// These tokens can start an expression after `!`, but
// can't continue an expression after an ident
token::Ident(ident, is_raw) => token::ident_can_begin_expr(ident, is_raw),
token::Literal(..) | token::Pound => true,
token::Interpolated(ref nt) => match nt.0 {
token::NtIdent(..) | token::NtExpr(..) |
token::NtBlock(..) | token::NtPath(..) => true,
_ => false,
},
_ => false
};
let cannot_continue_expr = self.look_ahead(1, token_cannot_continue_expr);
if cannot_continue_expr {
self.bump();
// Emit the error ...
let mut err = self.diagnostic()
.struct_span_err(self.span,
&format!("unexpected {} after identifier",
self.this_token_descr()));
// span the `not` plus trailing whitespace to avoid
// trailing whitespace after the `!` in our suggestion
let to_replace = self.sess.codemap()
.span_until_non_whitespace(lo.to(self.span));
err.span_suggestion_short(to_replace,
"use `!` to perform logical negation",
"!".to_owned());
err.emit();
// —and recover! (just as if we were in the block
// for the `token::Not` arm)
let e = self.parse_prefix_expr(None);
let (span, e) = self.interpolated_or_expr_span(e)?;
(lo.to(span), self.mk_unary(UnOp::Not, e))
} else {
return self.parse_dot_or_call_expr(Some(attrs));
}
}
_ => { return self.parse_dot_or_call_expr(Some(attrs)); }
};
return Ok(self.mk_expr(lo.to(hi), ex, attrs));
}
Expand Down
11 changes: 10 additions & 1 deletion src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl Lit {
}
}

fn ident_can_begin_expr(ident: ast::Ident, is_raw: bool) -> bool {
pub(crate) fn ident_can_begin_expr(ident: ast::Ident, is_raw: bool) -> bool {
let ident_token: Token = Ident(ident, is_raw);

!ident_token.is_reserved_ident() ||
Expand Down Expand Up @@ -348,6 +348,15 @@ impl Token {
self.lifetime().is_some()
}

/// Returns `true` if the token is a identifier whose name is the given
/// string slice.
pub fn is_ident_named(&self, name: &str) -> bool {
match self.ident() {
Some((ident, _)) => ident.name.as_str() == name,
None => false
}
}

/// Returns `true` if the token is a documentation comment.
pub fn is_doc_comment(&self) -> bool {
match *self {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn gratitude() {
let for_you = false;
if not for_you {
//~^ ERROR unexpected `for_you` after identifier
println!("I couldn't");
}
}

fn qualification() {
let the_worst = true;
while not the_worst {
//~^ ERROR unexpected `the_worst` after identifier
println!("still pretty bad");
}
}

fn should_we() {
let not = true;
if not // lack of braces is [sic]
println!("Then when?");
//~^ ERROR expected `{`, found `;
//~| ERROR unexpected `println` after identifier
}

fn sleepy() {
let resource = not 2;
//~^ ERROR unexpected `2` after identifier
}

fn main() {
let be_smothered_out_before = true;
let young_souls = not be_smothered_out_before;
//~^ ERROR unexpected `be_smothered_out_before` after identifier
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
error: unexpected `for_you` after identifier
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:13:12
|
LL | if not for_you {
| ----^^^^^^^
| |
| help: use `!` to perform logical negation

error: unexpected `the_worst` after identifier
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:21:15
|
LL | while not the_worst {
| ----^^^^^^^^^
| |
| help: use `!` to perform logical negation

error: unexpected `println` after identifier
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:30:9
|
LL | if not // lack of braces is [sic]
| ----- help: use `!` to perform logical negation
LL | println!("Then when?");
| ^^^^^^^

error: expected `{`, found `;`
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:30:31
|
LL | if not // lack of braces is [sic]
| -- this `if` statement has a condition, but no block
LL | println!("Then when?");
| ^

error: unexpected `2` after identifier
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:36:24
|
LL | let resource = not 2;
| ----^
| |
| help: use `!` to perform logical negation

error: unexpected `be_smothered_out_before` after identifier
--> $DIR/issue-46836-identifier-not-instead-of-negation.rs:42:27
|
LL | let young_souls = not be_smothered_out_before;
| ----^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: use `!` to perform logical negation

error: aborting due to 6 previous errors

0 comments on commit ba0dd8e

Please sign in to comment.