From b468b0e87c28e201427478bf6146afa0eaf85f81 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Thu, 8 Mar 2018 15:40:23 -0800 Subject: [PATCH] suggest `!` instead of erroneous `not` on if/while block parse failure Impressing confused Python users with magical diagnostics is probably worth this very slight (only 25ish lines) extra complexity in the parser? FIXME comments have been left to note that the formatting after autofixing the suggestion (with `rustfix` or an RLS-powered IDE) won't be optimal. (It doesn't look like any of the existing `CodeMap` methods make it easy to adjust the span.) Resolves #46836. --- src/libsyntax/parse/parser.rs | 30 +++++++- ...6836-identifier-not-instead-of-negation.rs | 45 ++++++++++++ ...-identifier-not-instead-of-negation.stderr | 68 +++++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.rs create mode 100644 src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index da2a22df997d1..4c9eb132cc2d5 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3211,6 +3211,21 @@ impl<'a> Parser<'a> { } } + /// If the condition of a `while` or `if` expression was an identifier + /// named `not`, that's a clue that the confused user likely meant `!`. + fn is_identifier_not(&self, cond: &Expr) -> bool { + if let ExprKind::Path(_, ref path) = cond.node { + if path.segments.len() == 1 { // just `not`, not `not::etc` + if let Some(segment) = path.segments.iter().next() { + if segment.identifier.name.as_str() == "not" { + return true; + } + } + } + } + false + } + /// Parse an 'if' or 'if let' expression ('if' token already eaten) pub fn parse_if_expr(&mut self, attrs: ThinVec) -> PResult<'a, P> { if self.check_keyword(keywords::Let) { @@ -3232,6 +3247,11 @@ impl<'a> Parser<'a> { } let not_block = self.token != token::OpenDelim(token::Brace); let thn = self.parse_block().map_err(|mut err| { + if self.is_identifier_not(&cond) { + err.span_suggestion(cond.span, // FIXME: replaced span doesn't incl. whitespace + "try replacing identifier `not` with the negation operator", + "!".to_owned()); + } if not_block { err.span_label(lo, "this `if` statement has a condition, but no block"); } @@ -3342,7 +3362,15 @@ impl<'a> Parser<'a> { return self.parse_while_let_expr(opt_label, span_lo, attrs); } let cond = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; - let (iattrs, body) = self.parse_inner_attrs_and_block()?; + let (iattrs, body) = self.parse_inner_attrs_and_block() + .map_err(|mut err| { + if self.is_identifier_not(&cond) { + let msg = "try replacing identifier `not` with the negation operator"; + // FIXME: replaced span doesn't include trailing whitespace + err.span_suggestion(cond.span, msg, "!".to_owned()); + } + err + })?; attrs.extend(iattrs); let span = span_lo.to(body.span); return Ok(self.mk_expr(span, ExprKind::While(cond, body, opt_label), attrs)); diff --git a/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.rs b/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.rs new file mode 100644 index 0000000000000..aa0a2ac4f6d48 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.rs @@ -0,0 +1,45 @@ +// 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 or the MIT license +// , 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 expected `{` + println!("I couldn't"); + //~^ ERROR expected one of + } +} + +fn qualification() { + let the_worst = true; + while not the_worst { + //~^ ERROR expected one of + println!("still pretty bad"); + } +} + +fn defer() { + let department = false; + // `not` as one segment of a longer path doesn't trigger the smart help + if not::my department { + //~^ ERROR expected `{` + println!("pass"); + //~^ ERROR expected one of + } +} + +fn should_we() { + let not = true; + if not // lack of braces is [sic] + println!("Then when?"); + //~^ ERROR expected `{` +} + +fn main() {} diff --git a/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr b/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr new file mode 100644 index 0000000000000..15c8d8ffe2e38 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr @@ -0,0 +1,68 @@ +error: expected one of `,` or `}`, found `!` + --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:15:16 + | +LL | println!("I couldn't"); + | ^ expected one of `,` or `}` here + +error: expected `{`, found `for_you` + --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:13:12 + | +LL | if not for_you { + | -- ^^^^^^^ + | | + | this `if` statement has a condition, but no block +help: try placing this code inside a block + | +LL | if not { for_you{println,}; } + | ^^^^^^^^^^^^^^^^^^^^^^ +help: try replacing identifier `not` with the negation operator + | +LL | if ! for_you { + | ^ + +error: expected one of `!`, `.`, `::`, `?`, `{`, or an operator, found `the_worst` + --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:22:15 + | +LL | while not the_worst { + | --- ^^^^^^^^^ expected one of `!`, `.`, `::`, `?`, `{`, or an operator here + | | + | help: try replacing identifier `not` with the negation operator: `!` + +error: expected one of `,` or `}`, found `!` + --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:33:16 + | +LL | println!("pass"); + | ^ expected one of `,` or `}` here + +error: expected `{`, found `department` + --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:31:16 + | +LL | if not::my department { + | -- -^^^^^^^^^ + | _____|__________| + | | | + | | this `if` statement has a condition, but no block +LL | | //~^ ERROR expected `{` +LL | | println!("pass"); +LL | | //~^ ERROR expected one of +LL | | } + | |_____- help: try placing this code inside a block: `{ department{println,}; }` + +error: expected `{`, found `println` + --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:41:9 + | +LL | if not // lack of braces is [sic] + | -- this `if` statement has a condition, but no block +LL | println!("Then when?"); + | ^^^^^^^ +help: try placing this code inside a block + | +LL | { println!("Then when?") } + | +help: try replacing identifier `not` with the negation operator + | +LL | if ! // lack of braces is [sic] + | ^ + +error: aborting due to 6 previous errors +