From 6a62ea682878e9f9e889c44a42f57a7eb1822e0d Mon Sep 17 00:00:00 2001 From: laurent Date: Mon, 30 Oct 2017 22:33:57 +0000 Subject: [PATCH 1/7] Add a nicer error message for missing in for loop, fixes #40782. --- src/libsyntax/parse/parser.rs | 54 ++++++++++++++++++++++++++++++---- src/test/ui/issue-40782.rs | 15 ++++++++++ src/test/ui/issue-40782.stderr | 11 +++++++ 3 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 src/test/ui/issue-40782.rs create mode 100644 src/test/ui/issue-40782.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a3a265450ab0e..37a12801ddcfc 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3154,13 +3154,55 @@ impl<'a> Parser<'a> { // Parse: `for in ` let pat = self.parse_pat()?; - self.expect_keyword(keywords::In)?; - let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; - let (iattrs, loop_block) = self.parse_inner_attrs_and_block()?; - attrs.extend(iattrs); + // Save the state of the parser before parsing 'in'. + let parser_snapshot_before_in = self.clone(); + match self.expect_keyword(keywords::In) { + Ok(()) => { + let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; + let (iattrs, loop_block) = self.parse_inner_attrs_and_block()?; + attrs.extend(iattrs); - let hi = self.prev_span; - Ok(self.mk_expr(span_lo.to(hi), ExprKind::ForLoop(pat, expr, loop_block, opt_ident), attrs)) + let hi = self.prev_span; + Ok(self.mk_expr( + span_lo.to(hi), + ExprKind::ForLoop(pat, expr, loop_block, opt_ident), + attrs)) + } + Err(mut in_err) => { + let parser_snapshot_after_in = self.clone(); + // Rewind to before attempting to parse the 'in'. + mem::replace(self, parser_snapshot_before_in); + + match self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None) { + Ok(expr) => { + // Successfully parsed the expr, print a nice error message. + in_err.cancel(); + let in_span = parser_snapshot_after_in.span; + let mut err = self.sess.span_diagnostic + .struct_span_err(in_span, "missing `in` in `for` loop"); + err.span_label(in_span, "expected `in` here"); + let sugg = pprust::to_string(|s| { + s.s.word("for ")?; + s.print_pat(&pat)?; + s.s.word(" in ")?; + s.print_expr(&expr) + }); + err.span_suggestion( + in_span, + "try adding `in`", + sugg); + Err(err) + } + Err(mut expr_err) => { + // Couldn't parse as an expr, return original error and parser state. + expr_err.cancel(); + mem::replace(self, parser_snapshot_after_in); + Err(in_err) + } + } + } + + } } /// Parse a 'while' or 'while let' expression ('while' token already eaten) diff --git a/src/test/ui/issue-40782.rs b/src/test/ui/issue-40782.rs new file mode 100644 index 0000000000000..56ee225105f89 --- /dev/null +++ b/src/test/ui/issue-40782.rs @@ -0,0 +1,15 @@ +// Copyright 2017 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 main() { + for i 0..2 { + } +} + diff --git a/src/test/ui/issue-40782.stderr b/src/test/ui/issue-40782.stderr new file mode 100644 index 0000000000000..a267c9bf4af78 --- /dev/null +++ b/src/test/ui/issue-40782.stderr @@ -0,0 +1,11 @@ +error: missing `in` in `for` loop + --> $DIR/issue-40782.rs:12:11 + | +12 | for i 0..2 { + | ^ + | | + | expected `in` here + | help: try adding `in`: `for i in 0..2` + +error: aborting due to previous error + From 6d060bd49a2a518d44deb36aadf772cc6ae61fb8 Mon Sep 17 00:00:00 2001 From: laurent Date: Tue, 31 Oct 2017 19:45:12 +0000 Subject: [PATCH 2/7] Fix spans and error messages. --- src/libsyntax/parse/parser.rs | 19 ++++++------------- src/test/ui/issue-40782.stderr | 10 +++++----- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 37a12801ddcfc..ffa6567c18bd0 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3174,23 +3174,16 @@ impl<'a> Parser<'a> { mem::replace(self, parser_snapshot_before_in); match self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None) { - Ok(expr) => { - // Successfully parsed the expr, print a nice error message. + Ok(_) => { + // Successfully parsed the expr which means that the 'in' keyword is + // missing, e.g. 'for i 0..2' in_err.cancel(); - let in_span = parser_snapshot_after_in.span; + let in_span = parser_snapshot_after_in.prev_span + .between(parser_snapshot_after_in.span); let mut err = self.sess.span_diagnostic .struct_span_err(in_span, "missing `in` in `for` loop"); err.span_label(in_span, "expected `in` here"); - let sugg = pprust::to_string(|s| { - s.s.word("for ")?; - s.print_pat(&pat)?; - s.s.word(" in ")?; - s.print_expr(&expr) - }); - err.span_suggestion( - in_span, - "try adding `in`", - sugg); + err.span_suggestion_short(in_span, "try adding `in` here", "in".into()); Err(err) } Err(mut expr_err) => { diff --git a/src/test/ui/issue-40782.stderr b/src/test/ui/issue-40782.stderr index a267c9bf4af78..ab87438555b28 100644 --- a/src/test/ui/issue-40782.stderr +++ b/src/test/ui/issue-40782.stderr @@ -1,11 +1,11 @@ error: missing `in` in `for` loop - --> $DIR/issue-40782.rs:12:11 + --> $DIR/issue-40782.rs:12:10 | 12 | for i 0..2 { - | ^ - | | - | expected `in` here - | help: try adding `in`: `for i in 0..2` + | ^ + | | + | expected `in` here + | help: try adding `in` here error: aborting due to previous error From 531b7f2e273dd00476482ead3cb0d24945190f64 Mon Sep 17 00:00:00 2001 From: laurent Date: Tue, 31 Oct 2017 21:23:46 +0000 Subject: [PATCH 3/7] Add some missing spaces. --- src/libsyntax/parse/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ffa6567c18bd0..1b0bdd8cb5e52 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3183,7 +3183,7 @@ impl<'a> Parser<'a> { let mut err = self.sess.span_diagnostic .struct_span_err(in_span, "missing `in` in `for` loop"); err.span_label(in_span, "expected `in` here"); - err.span_suggestion_short(in_span, "try adding `in` here", "in".into()); + err.span_suggestion_short(in_span, "try adding `in` here", " in ".into()); Err(err) } Err(mut expr_err) => { From 0d7285393f30d99e8715015bd1463c77acdc4690 Mon Sep 17 00:00:00 2001 From: laurent Date: Tue, 31 Oct 2017 21:26:49 +0000 Subject: [PATCH 4/7] Formatting tweak. --- src/libsyntax/parse/parser.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1b0bdd8cb5e52..1528ab25014da 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3194,8 +3194,7 @@ impl<'a> Parser<'a> { } } } - - } + } } /// Parse a 'while' or 'while let' expression ('while' token already eaten) From 175cfbf129866fb8412f79a02ab66ae549a24b10 Mon Sep 17 00:00:00 2001 From: laurent Date: Wed, 1 Nov 2017 06:45:34 +0000 Subject: [PATCH 5/7] Remove the parser snapshot hack. --- src/libsyntax/parse/parser.rs | 56 ++++++++++------------------------- 1 file changed, 16 insertions(+), 40 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1528ab25014da..4758417f4bd6a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3154,47 +3154,23 @@ impl<'a> Parser<'a> { // Parse: `for in ` let pat = self.parse_pat()?; - // Save the state of the parser before parsing 'in'. - let parser_snapshot_before_in = self.clone(); - match self.expect_keyword(keywords::In) { - Ok(()) => { - let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; - let (iattrs, loop_block) = self.parse_inner_attrs_and_block()?; - attrs.extend(iattrs); - - let hi = self.prev_span; - Ok(self.mk_expr( - span_lo.to(hi), - ExprKind::ForLoop(pat, expr, loop_block, opt_ident), - attrs)) - } - Err(mut in_err) => { - let parser_snapshot_after_in = self.clone(); - // Rewind to before attempting to parse the 'in'. - mem::replace(self, parser_snapshot_before_in); - - match self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None) { - Ok(_) => { - // Successfully parsed the expr which means that the 'in' keyword is - // missing, e.g. 'for i 0..2' - in_err.cancel(); - let in_span = parser_snapshot_after_in.prev_span - .between(parser_snapshot_after_in.span); - let mut err = self.sess.span_diagnostic - .struct_span_err(in_span, "missing `in` in `for` loop"); - err.span_label(in_span, "expected `in` here"); - err.span_suggestion_short(in_span, "try adding `in` here", " in ".into()); - Err(err) - } - Err(mut expr_err) => { - // Couldn't parse as an expr, return original error and parser state. - expr_err.cancel(); - mem::replace(self, parser_snapshot_after_in); - Err(in_err) - } - } - } + if !self.eat_keyword(keywords::In) { + let in_span = self.prev_span.between(self.span); + let mut err = self.sess.span_diagnostic + .struct_span_err(in_span, "missing `in` in `for` loop"); + err.span_label(in_span, "expected `in` here"); + err.span_suggestion_short(in_span, "try adding `in` here", " in ".into()); + err.emit(); } + let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; + let (iattrs, loop_block) = self.parse_inner_attrs_and_block()?; + attrs.extend(iattrs); + + let hi = self.prev_span; + Ok(self.mk_expr( + span_lo.to(hi), + ExprKind::ForLoop(pat, expr, loop_block, opt_ident), + attrs)) } /// Parse a 'while' or 'while let' expression ('while' token already eaten) From d336f022d56b7d1b0ed181de0c88b12bb9b2ae39 Mon Sep 17 00:00:00 2001 From: laurent Date: Wed, 1 Nov 2017 06:46:58 +0000 Subject: [PATCH 6/7] Preserve original formatting. --- src/libsyntax/parse/parser.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 4758417f4bd6a..ec9b22ae5a223 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3167,10 +3167,7 @@ impl<'a> Parser<'a> { attrs.extend(iattrs); let hi = self.prev_span; - Ok(self.mk_expr( - span_lo.to(hi), - ExprKind::ForLoop(pat, expr, loop_block, opt_ident), - attrs)) + Ok(self.mk_expr(span_lo.to(hi), ExprKind::ForLoop(pat, expr, loop_block, opt_ident), attrs)) } /// Parse a 'while' or 'while let' expression ('while' token already eaten) From ed20f3b5c0ae32802450c77da5c94c239c3a3500 Mon Sep 17 00:00:00 2001 From: laurent Date: Wed, 1 Nov 2017 23:43:32 +0000 Subject: [PATCH 7/7] Remove the redundant span_label. --- src/libsyntax/parse/parser.rs | 1 - src/test/ui/issue-40782.stderr | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ec9b22ae5a223..6de2ca5b9b874 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3158,7 +3158,6 @@ impl<'a> Parser<'a> { let in_span = self.prev_span.between(self.span); let mut err = self.sess.span_diagnostic .struct_span_err(in_span, "missing `in` in `for` loop"); - err.span_label(in_span, "expected `in` here"); err.span_suggestion_short(in_span, "try adding `in` here", " in ".into()); err.emit(); } diff --git a/src/test/ui/issue-40782.stderr b/src/test/ui/issue-40782.stderr index ab87438555b28..0d49eebbdbfc9 100644 --- a/src/test/ui/issue-40782.stderr +++ b/src/test/ui/issue-40782.stderr @@ -2,10 +2,7 @@ error: missing `in` in `for` loop --> $DIR/issue-40782.rs:12:10 | 12 | for i 0..2 { - | ^ - | | - | expected `in` here - | help: try adding `in` here + | ^ help: try adding `in` here error: aborting due to previous error