From 7969b78222dc66e21c6a21470365f404992729b1 Mon Sep 17 00:00:00 2001 From: Lamb Date: Tue, 29 May 2018 13:19:58 +0200 Subject: [PATCH 1/7] Issue #50974: Suboptimal error in case of duplicate `,` in struct constructor --- src/libsyntax/parse/parser.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 3955ccb4c420a..3d3867086454e 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -776,6 +776,9 @@ impl<'a> Parser<'a> { err.span_label(self.span, format!("expected identifier, found {}", token_descr)); } else { err.span_label(self.span, "expected identifier"); + if self.token == token::Comma { + err.span_suggestion(self.span, "try removing a comma", ",".into()); + } } err } @@ -2446,8 +2449,11 @@ impl<'a> Parser<'a> { Err(mut e) => { e.span_label(struct_sp, "while parsing this struct"); e.emit(); - self.recover_stmt(); - break; + + if self.token != token::Comma { + self.recover_stmt(); + break; + } } } From 783815d219413ae8475d0d37cb40e4139ac79f24 Mon Sep 17 00:00:00 2001 From: Lamb Date: Tue, 29 May 2018 20:15:47 +0200 Subject: [PATCH 2/7] Issue #50974: Change text of suggestion to be more direct --- 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 3d3867086454e..1ed902e88610d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -777,7 +777,7 @@ impl<'a> Parser<'a> { } else { err.span_label(self.span, "expected identifier"); if self.token == token::Comma { - err.span_suggestion(self.span, "try removing a comma", ",".into()); + err.span_suggestion(self.span, "remove this comma", ",".into()); } } err From 5c022af4a42fe50e641c679ea426cb562c68fc92 Mon Sep 17 00:00:00 2001 From: Lamb Date: Tue, 29 May 2018 20:15:59 +0200 Subject: [PATCH 3/7] Issue #50974: Adding tests --- src/test/ui/struct-duplicate-comma.rs | 25 +++++++++++++++++++++++ src/test/ui/struct-duplicate-comma.stderr | 13 ++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 src/test/ui/struct-duplicate-comma.rs create mode 100644 src/test/ui/struct-duplicate-comma.stderr diff --git a/src/test/ui/struct-duplicate-comma.rs b/src/test/ui/struct-duplicate-comma.rs new file mode 100644 index 0000000000000..87687d60e9690 --- /dev/null +++ b/src/test/ui/struct-duplicate-comma.rs @@ -0,0 +1,25 @@ +// 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. + +// compile-flags: -Z parse-only + +struct Foo { + a: u8, + b: u8 +} + +fn main() { + let bar = Foo { + a: 0,, + //~^ ERROR expected identifier + b: 42 + }; +} + diff --git a/src/test/ui/struct-duplicate-comma.stderr b/src/test/ui/struct-duplicate-comma.stderr new file mode 100644 index 0000000000000..c1a5f1754647a --- /dev/null +++ b/src/test/ui/struct-duplicate-comma.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `,` + --> $DIR/struct-duplicate-comma.rs:20:14 + | +LL | let bar = Foo { + | --- while parsing this struct +LL | a: 0,, + | ^ + | | + | expected identifier + | help: remove this comma: `,` + +error: aborting due to previous error + From 6c962e3ec413cb1cdb686c40609d4a1312f75e3d Mon Sep 17 00:00:00 2001 From: Lamb Date: Tue, 29 May 2018 21:44:55 +0200 Subject: [PATCH 4/7] Issue #50974: Adding issue number in the test --- src/test/ui/struct-duplicate-comma.rs | 2 ++ src/test/ui/struct-duplicate-comma.stderr | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/ui/struct-duplicate-comma.rs b/src/test/ui/struct-duplicate-comma.rs index 87687d60e9690..d7ee2f220d470 100644 --- a/src/test/ui/struct-duplicate-comma.rs +++ b/src/test/ui/struct-duplicate-comma.rs @@ -10,6 +10,8 @@ // compile-flags: -Z parse-only +// Issue #50974 + struct Foo { a: u8, b: u8 diff --git a/src/test/ui/struct-duplicate-comma.stderr b/src/test/ui/struct-duplicate-comma.stderr index c1a5f1754647a..37b573c2681e6 100644 --- a/src/test/ui/struct-duplicate-comma.stderr +++ b/src/test/ui/struct-duplicate-comma.stderr @@ -1,5 +1,5 @@ error: expected identifier, found `,` - --> $DIR/struct-duplicate-comma.rs:20:14 + --> $DIR/struct-duplicate-comma.rs:22:14 | LL | let bar = Foo { | --- while parsing this struct From fadb86f25d4053289c612cbba6b92da793976c16 Mon Sep 17 00:00:00 2001 From: Maerten <39694331+lambtowolf@users.noreply.github.com> Date: Wed, 30 May 2018 09:16:18 +0200 Subject: [PATCH 5/7] Fix when the help message is displayed Only display the "remove this comma" suggestion when followed by an identifier --- src/libsyntax/parse/parser.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1ed902e88610d..d6916c1c34460 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -776,8 +776,8 @@ impl<'a> Parser<'a> { err.span_label(self.span, format!("expected identifier, found {}", token_descr)); } else { err.span_label(self.span, "expected identifier"); - if self.token == token::Comma { - err.span_suggestion(self.span, "remove this comma", ",".into()); + if self.token == token::Comma && self.look_ahead(1, |t| *t.is_ident()) { + err.span_suggestion(self.span, "remove this comma", "".into()); } } err From 815765dadeb011180ad3ea0ce02c9cc59f8c93cc Mon Sep 17 00:00:00 2001 From: Lamb Date: Wed, 30 May 2018 13:06:05 +0200 Subject: [PATCH 6/7] Issue #50974: Fix compilation error and test --- src/libsyntax/parse/parser.rs | 2 +- src/test/ui/struct-duplicate-comma.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d6916c1c34460..456054ee25100 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -776,7 +776,7 @@ impl<'a> Parser<'a> { err.span_label(self.span, format!("expected identifier, found {}", token_descr)); } else { err.span_label(self.span, "expected identifier"); - if self.token == token::Comma && self.look_ahead(1, |t| *t.is_ident()) { + if self.token == token::Comma && self.look_ahead(1, |t| t.is_ident()) { err.span_suggestion(self.span, "remove this comma", "".into()); } } diff --git a/src/test/ui/struct-duplicate-comma.stderr b/src/test/ui/struct-duplicate-comma.stderr index 37b573c2681e6..06e3b95c24830 100644 --- a/src/test/ui/struct-duplicate-comma.stderr +++ b/src/test/ui/struct-duplicate-comma.stderr @@ -7,7 +7,7 @@ LL | a: 0,, | ^ | | | expected identifier - | help: remove this comma: `,` + | help: remove this comma error: aborting due to previous error From a99767f64f400f694aec02655af4b5cb5913609e Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 5 Jun 2018 13:04:15 -0400 Subject: [PATCH 7/7] add an explanatory comment for recovery behavior --- src/libsyntax/parse/parser.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 456054ee25100..dfd896fbda449 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2450,6 +2450,9 @@ impl<'a> Parser<'a> { e.span_label(struct_sp, "while parsing this struct"); e.emit(); + // If the next token is a comma, then try to parse + // what comes next as additional fields, rather than + // bailing out until next `}`. if self.token != token::Comma { self.recover_stmt(); break;