From 03eca713816ee00ecacde27cc655dc199c6bff40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Mar 2017 19:14:58 -0700 Subject: [PATCH 1/3] Point at last valid token on failed `expect_one_of` ```rust error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` --> $DIR/token-error-correct-3.rs:29:9 | 25 | foo() | - expected one of `.`, `;`, `?`, `}`, or an operator after this ... 29 | } else { | ^ unexpected token ``` --- src/libsyntax/parse/parser.rs | 28 +++++++++---------- src/test/compile-fail/issue-10636-2.rs | 2 ++ .../compile-fail/macro-incomplete-parse.rs | 2 ++ src/test/parse-fail/bounds-obj-parens.rs | 4 ++- src/test/parse-fail/match-refactor-to-expr.rs | 4 ++- .../parse-fail/trailing-plus-in-bounds.rs | 4 ++- .../ui/resolve/token-error-correct-3.stderr | 9 ++++-- .../ui/resolve/token-error-correct.stderr | 4 ++- 8 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index df4ccc94c0421..6379015055b60 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -548,20 +548,20 @@ impl<'a> Parser<'a> { expected.dedup(); let expect = tokens_to_string(&expected[..]); let actual = self.this_token_to_string(); - Err(self.fatal( - &(if expected.len() > 1 { - (format!("expected one of {}, found `{}`", - expect, - actual)) - } else if expected.is_empty() { - (format!("unexpected token: `{}`", - actual)) - } else { - (format!("expected {}, found `{}`", - expect, - actual)) - })[..] - )) + let (msg_exp, label_exp) = if expected.len() > 1 { + (format!("expected one of {}, found `{}`", expect, actual), + format!("expected one of {} after this", expect)) + } else if expected.is_empty() { + (format!("unexpected token: `{}`", actual), + "unexpected token after this".to_string()) + } else { + (format!("expected {}, found `{}`", expect, actual), + format!("expected {} after this", expect)) + }; + let mut err = self.fatal(&msg_exp); + err.span_label(self.prev_span, &label_exp); + err.span_label(self.span, &"unexpected token"); + Err(err) } } diff --git a/src/test/compile-fail/issue-10636-2.rs b/src/test/compile-fail/issue-10636-2.rs index beaf9e5059fa2..93759123618fb 100644 --- a/src/test/compile-fail/issue-10636-2.rs +++ b/src/test/compile-fail/issue-10636-2.rs @@ -14,5 +14,7 @@ pub fn trace_option(option: Option) { option.map(|some| 42; //~ NOTE: unclosed delimiter //~^ ERROR: expected one of + //~| NOTE: expected one of + //~| NOTE: unexpected token } //~ ERROR: incorrect close delimiter //~^ ERROR: expected expression, found `)` diff --git a/src/test/compile-fail/macro-incomplete-parse.rs b/src/test/compile-fail/macro-incomplete-parse.rs index c2ac99d1f6a2d..682664df98109 100644 --- a/src/test/compile-fail/macro-incomplete-parse.rs +++ b/src/test/compile-fail/macro-incomplete-parse.rs @@ -20,6 +20,8 @@ macro_rules! ignored_item { macro_rules! ignored_expr { () => ( 1, //~ ERROR expected one of `.`, `;`, `?`, `}`, or an operator, found `,` + //~^ NOTE expected one of `.`, `;`, `?`, `}`, or an operator after this + //~| NOTE unexpected token 2 ) } diff --git a/src/test/parse-fail/bounds-obj-parens.rs b/src/test/parse-fail/bounds-obj-parens.rs index ad59d4a52d74c..02c119cf727fe 100644 --- a/src/test/parse-fail/bounds-obj-parens.rs +++ b/src/test/parse-fail/bounds-obj-parens.rs @@ -12,4 +12,6 @@ type A = Box<(Fn(D::Error) -> E) + 'static + Send + Sync>; // OK (but see #39318) -FAIL //~ ERROR +FAIL +//~^ ERROR +//~| ERROR diff --git a/src/test/parse-fail/match-refactor-to-expr.rs b/src/test/parse-fail/match-refactor-to-expr.rs index 37b66601e7092..7bb1c40118a4d 100644 --- a/src/test/parse-fail/match-refactor-to-expr.rs +++ b/src/test/parse-fail/match-refactor-to-expr.rs @@ -14,7 +14,9 @@ fn main() { let foo = match //~ NOTE did you mean to remove this `match` keyword? Some(4).unwrap_or_else(5) - ; //~ ERROR expected one of `.`, `?`, `{`, or an operator, found `;` + //~^ NOTE expected one of `.`, `?`, `{`, or an operator after this + ; //~ NOTE unexpected token + //~^ ERROR expected one of `.`, `?`, `{`, or an operator, found `;` println!("{}", foo) } diff --git a/src/test/parse-fail/trailing-plus-in-bounds.rs b/src/test/parse-fail/trailing-plus-in-bounds.rs index 4a2e6d5bdcd9c..2bb2c97790c12 100644 --- a/src/test/parse-fail/trailing-plus-in-bounds.rs +++ b/src/test/parse-fail/trailing-plus-in-bounds.rs @@ -16,4 +16,6 @@ fn main() { let x: Box = box 3 as Box; // Trailing `+` is OK } -FAIL //~ ERROR +FAIL +//~^ ERROR +//~| ERROR diff --git a/src/test/ui/resolve/token-error-correct-3.stderr b/src/test/ui/resolve/token-error-correct-3.stderr index 56e3688957502..2e0edf0c4b8d1 100644 --- a/src/test/ui/resolve/token-error-correct-3.stderr +++ b/src/test/ui/resolve/token-error-correct-3.stderr @@ -14,13 +14,18 @@ error: expected one of `,`, `.`, `?`, or an operator, found `;` --> $DIR/token-error-correct-3.rs:23:35 | 23 | callback(path.as_ref(); //~ NOTE: unclosed delimiter - | ^ + | -^ unexpected token + | | + | expected one of `,`, `.`, `?`, or an operator after this error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` --> $DIR/token-error-correct-3.rs:29:9 | +25 | fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: mismatched types + | - expected one of `.`, `;`, `?`, `}`, or an operator after this +... 29 | } else { //~ ERROR: incorrect close delimiter: `}` - | ^ + | ^ unexpected token error[E0425]: cannot find function `is_directory` in this scope --> $DIR/token-error-correct-3.rs:21:13 diff --git a/src/test/ui/resolve/token-error-correct.stderr b/src/test/ui/resolve/token-error-correct.stderr index 248a923efaf36..36f298a456a60 100644 --- a/src/test/ui/resolve/token-error-correct.stderr +++ b/src/test/ui/resolve/token-error-correct.stderr @@ -32,7 +32,9 @@ error: expected one of `)`, `,`, `.`, `<`, `?`, `break`, `continue`, `false`, `f --> $DIR/token-error-correct.rs:14:13 | 14 | foo(bar(; - | ^ + | -^ unexpected token + | | + | expected one of `)`, `,`, `.`, `<`, `?`, `break`, `continue`, `false`, `for`, `if`, `loop`, `match`, `move`, `return`, `true`, `unsafe`, `while`, or an operator after this error: expected expression, found `)` --> $DIR/token-error-correct.rs:23:1 From 78ae8feebbf9a2c70d42780d0c646cbbc1f2cdbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 Mar 2017 15:36:59 -0700 Subject: [PATCH 2/3] Improve wording and spans for unexpected token * Point at where the token was expected instead of the last token successfuly parsed. * Only show `unexpected token` if the next char and the unexpected token don't have the same span. * Change some cfail and pfail tests to ui test. * Don't show all possible tokens in span label if they are more than 6. --- src/libsyntax/parse/parser.rs | 25 +++++++++++---- src/libsyntax_pos/lib.rs | 6 ++++ src/test/parse-fail/match-refactor-to-expr.rs | 2 +- .../ui/resolve/token-error-correct-3.stderr | 6 ++-- .../ui/resolve/token-error-correct.stderr | 4 +-- .../token}/bounds-obj-parens.rs | 0 src/test/ui/token/bounds-obj-parens.stderr | 7 +++++ .../token}/issue-10636-2.rs | 0 src/test/ui/token/issue-10636-2.stderr | 27 ++++++++++++++++ .../token}/macro-incomplete-parse.rs | 2 +- .../ui/token/macro-incomplete-parse.stderr | 31 +++++++++++++++++++ .../token}/trailing-plus-in-bounds.rs | 0 .../ui/token/trailing-plus-in-bounds.stderr | 7 +++++ 13 files changed, 102 insertions(+), 15 deletions(-) rename src/test/{parse-fail => ui/token}/bounds-obj-parens.rs (100%) create mode 100644 src/test/ui/token/bounds-obj-parens.stderr rename src/test/{compile-fail => ui/token}/issue-10636-2.rs (100%) create mode 100644 src/test/ui/token/issue-10636-2.stderr rename src/test/{compile-fail => ui/token}/macro-incomplete-parse.rs (98%) create mode 100644 src/test/ui/token/macro-incomplete-parse.stderr rename src/test/{parse-fail => ui/token}/trailing-plus-in-bounds.rs (100%) create mode 100644 src/test/ui/token/trailing-plus-in-bounds.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 6379015055b60..4076368c180b6 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -548,19 +548,32 @@ impl<'a> Parser<'a> { expected.dedup(); let expect = tokens_to_string(&expected[..]); let actual = self.this_token_to_string(); - let (msg_exp, label_exp) = if expected.len() > 1 { + let (msg_exp, (label_sp, label_exp)) = if expected.len() > 1 { + let short_expect = if expected.len() > 6 { + format!("{} possible tokens", expected.len()) + } else { + expect.clone() + }; (format!("expected one of {}, found `{}`", expect, actual), - format!("expected one of {} after this", expect)) + (self.prev_span.next_point(), format!("expected one of {} here", short_expect))) } else if expected.is_empty() { (format!("unexpected token: `{}`", actual), - "unexpected token after this".to_string()) + (self.prev_span, "unexpected token after this".to_string())) } else { (format!("expected {}, found `{}`", expect, actual), - format!("expected {} after this", expect)) + (self.prev_span.next_point(), format!("expected {} here", expect))) }; let mut err = self.fatal(&msg_exp); - err.span_label(self.prev_span, &label_exp); - err.span_label(self.span, &"unexpected token"); + let sp = if self.token == token::Token::Eof { + // This is EOF, don't want to point at the following char, but rather the last token + self.prev_span + } else { + label_sp + }; + err.span_label(sp, &label_exp); + if label_sp != self.span { + err.span_label(self.span, &"unexpected token"); + } Err(err) } } diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 3808923e7728f..07494ff904e95 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -79,6 +79,12 @@ impl Span { Span { lo: BytePos(lo), hi: self.hi, expn_id: self.expn_id} } + /// Returns a new span representing the next character after the end-point of this span + pub fn next_point(self) -> Span { + let lo = BytePos(cmp::max(self.hi.0, self.lo.0 + 1)); + Span { lo: lo, hi: lo, expn_id: self.expn_id} + } + /// Returns `self` if `self` is not the dummy span, and `other` otherwise. pub fn substitute_dummy(self, other: Span) -> Span { if self.source_equal(&DUMMY_SP) { other } else { self } diff --git a/src/test/parse-fail/match-refactor-to-expr.rs b/src/test/parse-fail/match-refactor-to-expr.rs index 7bb1c40118a4d..e2fee1d189591 100644 --- a/src/test/parse-fail/match-refactor-to-expr.rs +++ b/src/test/parse-fail/match-refactor-to-expr.rs @@ -14,7 +14,7 @@ fn main() { let foo = match //~ NOTE did you mean to remove this `match` keyword? Some(4).unwrap_or_else(5) - //~^ NOTE expected one of `.`, `?`, `{`, or an operator after this + //~^ NOTE expected one of `.`, `?`, `{`, or an operator here ; //~ NOTE unexpected token //~^ ERROR expected one of `.`, `?`, `{`, or an operator, found `;` diff --git a/src/test/ui/resolve/token-error-correct-3.stderr b/src/test/ui/resolve/token-error-correct-3.stderr index 2e0edf0c4b8d1..bf7db67e72850 100644 --- a/src/test/ui/resolve/token-error-correct-3.stderr +++ b/src/test/ui/resolve/token-error-correct-3.stderr @@ -14,15 +14,13 @@ error: expected one of `,`, `.`, `?`, or an operator, found `;` --> $DIR/token-error-correct-3.rs:23:35 | 23 | callback(path.as_ref(); //~ NOTE: unclosed delimiter - | -^ unexpected token - | | - | expected one of `,`, `.`, `?`, or an operator after this + | ^ expected one of `,`, `.`, `?`, or an operator here error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` --> $DIR/token-error-correct-3.rs:29:9 | 25 | fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: mismatched types - | - expected one of `.`, `;`, `?`, `}`, or an operator after this + | - expected one of `.`, `;`, `?`, `}`, or an operator here ... 29 | } else { //~ ERROR: incorrect close delimiter: `}` | ^ unexpected token diff --git a/src/test/ui/resolve/token-error-correct.stderr b/src/test/ui/resolve/token-error-correct.stderr index 36f298a456a60..226fa6469bc74 100644 --- a/src/test/ui/resolve/token-error-correct.stderr +++ b/src/test/ui/resolve/token-error-correct.stderr @@ -32,9 +32,7 @@ error: expected one of `)`, `,`, `.`, `<`, `?`, `break`, `continue`, `false`, `f --> $DIR/token-error-correct.rs:14:13 | 14 | foo(bar(; - | -^ unexpected token - | | - | expected one of `)`, `,`, `.`, `<`, `?`, `break`, `continue`, `false`, `for`, `if`, `loop`, `match`, `move`, `return`, `true`, `unsafe`, `while`, or an operator after this + | ^ expected one of 18 possible tokens here error: expected expression, found `)` --> $DIR/token-error-correct.rs:23:1 diff --git a/src/test/parse-fail/bounds-obj-parens.rs b/src/test/ui/token/bounds-obj-parens.rs similarity index 100% rename from src/test/parse-fail/bounds-obj-parens.rs rename to src/test/ui/token/bounds-obj-parens.rs diff --git a/src/test/ui/token/bounds-obj-parens.stderr b/src/test/ui/token/bounds-obj-parens.stderr new file mode 100644 index 0000000000000..ebee363f278e2 --- /dev/null +++ b/src/test/ui/token/bounds-obj-parens.stderr @@ -0,0 +1,7 @@ +error: expected one of `!` or `::`, found `` + --> $DIR/bounds-obj-parens.rs:15:1 + | +15 | FAIL + | ^^^^ expected one of `!` or `::` here + +error: aborting due to previous error diff --git a/src/test/compile-fail/issue-10636-2.rs b/src/test/ui/token/issue-10636-2.rs similarity index 100% rename from src/test/compile-fail/issue-10636-2.rs rename to src/test/ui/token/issue-10636-2.rs diff --git a/src/test/ui/token/issue-10636-2.stderr b/src/test/ui/token/issue-10636-2.stderr new file mode 100644 index 0000000000000..183ad30c4ef47 --- /dev/null +++ b/src/test/ui/token/issue-10636-2.stderr @@ -0,0 +1,27 @@ +error: incorrect close delimiter: `}` + --> $DIR/issue-10636-2.rs:19:1 + | +19 | } //~ ERROR: incorrect close delimiter + | ^ + | +note: unclosed delimiter + --> $DIR/issue-10636-2.rs:15:15 + | +15 | option.map(|some| 42; //~ NOTE: unclosed delimiter + | ^ + +error: expected one of `,`, `.`, `?`, or an operator, found `;` + --> $DIR/issue-10636-2.rs:15:25 + | +15 | option.map(|some| 42; //~ NOTE: unclosed delimiter + | ^ expected one of `,`, `.`, `?`, or an operator here + +error: expected expression, found `)` + --> $DIR/issue-10636-2.rs:19:1 + | +19 | } //~ ERROR: incorrect close delimiter + | ^ + +error: main function not found + +error: aborting due to 4 previous errors diff --git a/src/test/compile-fail/macro-incomplete-parse.rs b/src/test/ui/token/macro-incomplete-parse.rs similarity index 98% rename from src/test/compile-fail/macro-incomplete-parse.rs rename to src/test/ui/token/macro-incomplete-parse.rs index 682664df98109..47374fc3c6085 100644 --- a/src/test/compile-fail/macro-incomplete-parse.rs +++ b/src/test/ui/token/macro-incomplete-parse.rs @@ -20,7 +20,7 @@ macro_rules! ignored_item { macro_rules! ignored_expr { () => ( 1, //~ ERROR expected one of `.`, `;`, `?`, `}`, or an operator, found `,` - //~^ NOTE expected one of `.`, `;`, `?`, `}`, or an operator after this + //~^ NOTE expected one of `.`, `;`, `?`, `}`, or an operator here //~| NOTE unexpected token 2 ) } diff --git a/src/test/ui/token/macro-incomplete-parse.stderr b/src/test/ui/token/macro-incomplete-parse.stderr new file mode 100644 index 0000000000000..bea00a6444c47 --- /dev/null +++ b/src/test/ui/token/macro-incomplete-parse.stderr @@ -0,0 +1,31 @@ +error: macro expansion ignores token `,` and any following + --> $DIR/macro-incomplete-parse.rs:17:9 + | +17 | , //~ ERROR macro expansion ignores token `,` + | ^ + | +note: caused by the macro expansion here; the usage of `ignored_item!` is likely invalid in item context + --> $DIR/macro-incomplete-parse.rs:32:1 + | +32 | ignored_item!(); //~ NOTE caused by the macro expansion here + | ^^^^^^^^^^^^^^^^ + +error: expected one of `.`, `;`, `?`, `}`, or an operator, found `,` + --> $DIR/macro-incomplete-parse.rs:22:14 + | +22 | () => ( 1, //~ ERROR expected one of `.`, `;`, `?`, `}`, or an operator, found `,` + | ^ expected one of `.`, `;`, `?`, `}`, or an operator here + +error: macro expansion ignores token `,` and any following + --> $DIR/macro-incomplete-parse.rs:29:14 + | +29 | () => ( 1, 2 ) //~ ERROR macro expansion ignores token `,` + | ^ + | +note: caused by the macro expansion here; the usage of `ignored_pat!` is likely invalid in pattern context + --> $DIR/macro-incomplete-parse.rs:37:9 + | +37 | ignored_pat!() => (), //~ NOTE caused by the macro expansion here + | ^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors diff --git a/src/test/parse-fail/trailing-plus-in-bounds.rs b/src/test/ui/token/trailing-plus-in-bounds.rs similarity index 100% rename from src/test/parse-fail/trailing-plus-in-bounds.rs rename to src/test/ui/token/trailing-plus-in-bounds.rs diff --git a/src/test/ui/token/trailing-plus-in-bounds.stderr b/src/test/ui/token/trailing-plus-in-bounds.stderr new file mode 100644 index 0000000000000..74caf8f5c2b36 --- /dev/null +++ b/src/test/ui/token/trailing-plus-in-bounds.stderr @@ -0,0 +1,7 @@ +error: expected one of `!` or `::`, found `` + --> ../../src/test/ui/token/trailing-plus-in-bounds.rs:19:1 + | +19 | FAIL + | ^^^^ expected one of `!` or `::` here + +error: aborting due to previous error From b477682dca3343eb89a467f0d3c73986a53d49d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 Mar 2017 19:06:19 -0700 Subject: [PATCH 3/3] Fix unittests --- src/libsyntax/parse/parser.rs | 2 +- src/libsyntax_pos/lib.rs | 4 ++-- src/test/ui/token/bounds-obj-parens.stderr | 1 + src/test/ui/token/issue-10636-2.stderr | 1 + src/test/ui/token/macro-incomplete-parse.stderr | 1 + src/test/ui/token/trailing-plus-in-bounds.stderr | 3 ++- 6 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 4076368c180b6..8177d738dc834 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -571,7 +571,7 @@ impl<'a> Parser<'a> { label_sp }; err.span_label(sp, &label_exp); - if label_sp != self.span { + if !sp.source_equal(&self.span) { err.span_label(self.span, &"unexpected token"); } Err(err) diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 07494ff904e95..0662c1c9cfdc8 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -81,8 +81,8 @@ impl Span { /// Returns a new span representing the next character after the end-point of this span pub fn next_point(self) -> Span { - let lo = BytePos(cmp::max(self.hi.0, self.lo.0 + 1)); - Span { lo: lo, hi: lo, expn_id: self.expn_id} + let lo = cmp::max(self.hi.0, self.lo.0 + 1); + Span { lo: BytePos(lo), hi: BytePos(lo + 1), expn_id: self.expn_id} } /// Returns `self` if `self` is not the dummy span, and `other` otherwise. diff --git a/src/test/ui/token/bounds-obj-parens.stderr b/src/test/ui/token/bounds-obj-parens.stderr index ebee363f278e2..4d60be15ecaf0 100644 --- a/src/test/ui/token/bounds-obj-parens.stderr +++ b/src/test/ui/token/bounds-obj-parens.stderr @@ -5,3 +5,4 @@ error: expected one of `!` or `::`, found `` | ^^^^ expected one of `!` or `::` here error: aborting due to previous error + diff --git a/src/test/ui/token/issue-10636-2.stderr b/src/test/ui/token/issue-10636-2.stderr index 183ad30c4ef47..b0bae1248b969 100644 --- a/src/test/ui/token/issue-10636-2.stderr +++ b/src/test/ui/token/issue-10636-2.stderr @@ -25,3 +25,4 @@ error: expected expression, found `)` error: main function not found error: aborting due to 4 previous errors + diff --git a/src/test/ui/token/macro-incomplete-parse.stderr b/src/test/ui/token/macro-incomplete-parse.stderr index bea00a6444c47..f23d97586b843 100644 --- a/src/test/ui/token/macro-incomplete-parse.stderr +++ b/src/test/ui/token/macro-incomplete-parse.stderr @@ -29,3 +29,4 @@ note: caused by the macro expansion here; the usage of `ignored_pat!` is likely | ^^^^^^^^^^^^^^ error: aborting due to 3 previous errors + diff --git a/src/test/ui/token/trailing-plus-in-bounds.stderr b/src/test/ui/token/trailing-plus-in-bounds.stderr index 74caf8f5c2b36..c765a434b8ac6 100644 --- a/src/test/ui/token/trailing-plus-in-bounds.stderr +++ b/src/test/ui/token/trailing-plus-in-bounds.stderr @@ -1,7 +1,8 @@ error: expected one of `!` or `::`, found `` - --> ../../src/test/ui/token/trailing-plus-in-bounds.rs:19:1 + --> $DIR/trailing-plus-in-bounds.rs:19:1 | 19 | FAIL | ^^^^ expected one of `!` or `::` here error: aborting due to previous error +