Skip to content

Commit 1f04597

Browse files
committed
in which parentheses are suggested for should-have-been-tuple-patterns
Programmers used to working in some other languages (such as Python or Go) might expect to be able to destructure values with comma-separated identifiers but no parentheses on the left side of an assignment. Previously, the first name in such code would get parsed as a single-indentifier pattern—recognizing, for example, the `let a` in `let a, b = (1, 2);`—whereupon we would have a fatal syntax error on seeing an unexpected comma rather than the expected semicolon (all the way nearer to the end of `parse_full_stmt`). Instead, let's look for that comma when parsing the pattern, and if we see it, momentarily make-believe that we're parsing the remaining elements in a tuple pattern, so that we can suggest wrapping it all in parentheses. We need to do this in a separate wrapper method called on the top-level pattern (or `|`-patterns) in a `let` statement, `for` loop, `if`- or `while let` expression, or match arm rather than within `parse_pat` itself, because `parse_pat` gets called recursively to parse the sub-patterns within a tuple pattern. Resolves #48492.
1 parent c90f682 commit 1f04597

File tree

3 files changed

+174
-5
lines changed

3 files changed

+174
-5
lines changed

src/libsyntax/parse/parser.rs

+39-5
Original file line numberDiff line numberDiff line change
@@ -3318,7 +3318,7 @@ impl<'a> Parser<'a> {
33183318
mut attrs: ThinVec<Attribute>) -> PResult<'a, P<Expr>> {
33193319
// Parse: `for <src_pat> in <src_expr> <src_loop_block>`
33203320

3321-
let pat = self.parse_pat()?;
3321+
let pat = self.parse_top_level_pat()?;
33223322
if !self.eat_keyword(keywords::In) {
33233323
let in_span = self.prev_span.between(self.span);
33243324
let mut err = self.sess.span_diagnostic
@@ -3528,7 +3528,7 @@ impl<'a> Parser<'a> {
35283528
fn parse_pats(&mut self) -> PResult<'a, Vec<P<Pat>>> {
35293529
let mut pats = Vec::new();
35303530
loop {
3531-
pats.push(self.parse_pat()?);
3531+
pats.push(self.parse_top_level_pat()?);
35323532

35333533
if self.token == token::OrOr {
35343534
let mut err = self.struct_span_err(self.span,
@@ -3554,7 +3554,12 @@ impl<'a> Parser<'a> {
35543554
// Trailing commas are significant because (p) and (p,) are different patterns.
35553555
fn parse_parenthesized_pat_list(&mut self) -> PResult<'a, (Vec<P<Pat>>, Option<usize>, bool)> {
35563556
self.expect(&token::OpenDelim(token::Paren))?;
3557+
let result = self.parse_pat_list()?;
3558+
self.expect(&token::CloseDelim(token::Paren))?;
3559+
Ok(result)
3560+
}
35573561

3562+
fn parse_pat_list(&mut self) -> PResult<'a, (Vec<P<Pat>>, Option<usize>, bool)> {
35583563
let mut fields = Vec::new();
35593564
let mut ddpos = None;
35603565
let mut trailing_comma = false;
@@ -3584,8 +3589,6 @@ impl<'a> Parser<'a> {
35843589
self.span_err(self.prev_span, "trailing comma is not permitted after `..`");
35853590
}
35863591

3587-
self.expect(&token::CloseDelim(token::Paren))?;
3588-
35893592
Ok((fields, ddpos, trailing_comma))
35903593
}
35913594

@@ -3767,6 +3770,37 @@ impl<'a> Parser<'a> {
37673770
}))
37683771
}
37693772

3773+
/// A wrapper around `parse_pat` with some special error handling for the
3774+
/// "top-level" patterns in a match arm, `for` loop, `let`, &c. (in contast
3775+
/// to subpatterns within such).
3776+
pub fn parse_top_level_pat(&mut self) -> PResult<'a, P<Pat>> {
3777+
let pat = self.parse_pat()?;
3778+
if self.token == token::Comma {
3779+
// An unexpected comma after a top-level pattern is a clue that the
3780+
// user (perhaps more accustomed to some other language) forgot the
3781+
// parentheses in what should have been a tuple pattern; return a
3782+
// suggestion-enhanced error here rather than choking on the comma
3783+
// later.
3784+
let comma_span = self.span;
3785+
self.bump();
3786+
if let Err(mut err) = self.parse_pat_list() {
3787+
// We didn't expect this to work anyway; we just wanted
3788+
// to advance to the end of the comma-sequence so we know
3789+
// the span to suggest parenthesizing
3790+
err.cancel();
3791+
}
3792+
let seq_span = pat.span.to(self.prev_span);
3793+
let mut err = self.struct_span_err(comma_span,
3794+
"unexpected `,` in pattern");
3795+
if let Ok(seq_snippet) = self.sess.codemap().span_to_snippet(seq_span) {
3796+
err.span_suggestion(seq_span, "try adding parentheses",
3797+
format!("({})", seq_snippet));
3798+
}
3799+
return Err(err);
3800+
}
3801+
Ok(pat)
3802+
}
3803+
37703804
/// Parse a pattern.
37713805
pub fn parse_pat(&mut self) -> PResult<'a, P<Pat>> {
37723806
maybe_whole!(self, NtPat, |x| x);
@@ -3969,7 +4003,7 @@ impl<'a> Parser<'a> {
39694003
/// Parse a local variable declaration
39704004
fn parse_local(&mut self, attrs: ThinVec<Attribute>) -> PResult<'a, P<Local>> {
39714005
let lo = self.prev_span;
3972-
let pat = self.parse_pat()?;
4006+
let pat = self.parse_top_level_pat()?;
39734007

39744008
let (err, ty) = if self.eat(&token::Colon) {
39754009
// Save the state of the parser before parsing type normally, in case there is a `:`
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![allow(unused)]
12+
13+
#[derive(Copy, Clone)]
14+
enum Nucleotide {
15+
Adenine,
16+
Thymine,
17+
Cytosine,
18+
Guanine
19+
}
20+
21+
#[derive(Clone)]
22+
struct Autosome;
23+
24+
#[derive(Clone)]
25+
enum Allosome {
26+
X(Vec<Nucleotide>),
27+
Y(Vec<Nucleotide>)
28+
}
29+
30+
impl Allosome {
31+
fn is_x(&self) -> bool {
32+
match *self {
33+
Allosome::X(_) => true,
34+
Allosome::Y(_) => false,
35+
}
36+
}
37+
}
38+
39+
#[derive(Clone)]
40+
struct Genome {
41+
autosomes: [Autosome; 22],
42+
allosomes: (Allosome, Allosome)
43+
}
44+
45+
fn find_start_codon(strand: &[Nucleotide]) -> Option<usize> {
46+
let mut reading_frame = strand.windows(3);
47+
// (missing parentheses in `while let` tuple pattern)
48+
while let b1, b2, b3 = reading_frame.next().expect("there should be a start codon") {
49+
//~^ ERROR unexpected `,` in pattern
50+
// ...
51+
}
52+
None
53+
}
54+
55+
fn find_thr(strand: &[Nucleotide]) -> Option<usize> {
56+
let mut reading_frame = strand.windows(3);
57+
let mut i = 0;
58+
// (missing parentheses in `if let` tuple pattern)
59+
if let b1, b2, b3 = reading_frame.next().unwrap() {
60+
//~^ ERROR unexpected `,` in pattern
61+
// ...
62+
}
63+
None
64+
}
65+
66+
fn is_thr(codon: (Nucleotide, Nucleotide, Nucleotide)) -> bool {
67+
match codon {
68+
// (missing parentheses in match arm tuple pattern)
69+
Nucleotide::Adenine, Nucleotide::Cytosine, _ => true
70+
//~^ ERROR unexpected `,` in pattern
71+
_ => false
72+
}
73+
}
74+
75+
fn analyze_female_sex_chromosomes(women: &[Genome]) {
76+
// (missing parentheses in `for` tuple pattern)
77+
for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) {
78+
//~^ ERROR unexpected `,` in pattern
79+
// ...
80+
}
81+
}
82+
83+
fn analyze_male_sex_chromosomes(men: &[Genome]) {
84+
// (missing parentheses in pattern with `@` binding)
85+
for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) {
86+
//~^ ERROR unexpected `,` in pattern
87+
// ...
88+
}
89+
}
90+
91+
fn main() {
92+
let genomes = Vec::new();
93+
// (missing parentheses in `let` pattern)
94+
let women, men: (Vec<Genome>, Vec<Genome>) = genomes.iter().cloned()
95+
//~^ ERROR unexpected `,` in pattern
96+
.partition(|g: &Genome| g.allosomes.0.is_x() && g.allosomes.1.is_x());
97+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
error: unexpected `,` in pattern
2+
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:48:17
3+
|
4+
LL | while let b1, b2, b3 = reading_frame.next().expect("there should be a start codon") {
5+
| --^------- help: try adding parentheses: `(b1, b2, b3)`
6+
7+
error: unexpected `,` in pattern
8+
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:59:14
9+
|
10+
LL | if let b1, b2, b3 = reading_frame.next().unwrap() {
11+
| --^------- help: try adding parentheses: `(b1, b2, b3)`
12+
13+
error: unexpected `,` in pattern
14+
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:69:28
15+
|
16+
LL | Nucleotide::Adenine, Nucleotide::Cytosine, _ => true
17+
| -------------------^------------------------ help: try adding parentheses: `(Nucleotide::Adenine, Nucleotide::Cytosine, _)`
18+
19+
error: unexpected `,` in pattern
20+
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:77:10
21+
|
22+
LL | for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) {
23+
| -^----------- help: try adding parentheses: `(x, _barr_body)`
24+
25+
error: unexpected `,` in pattern
26+
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:85:10
27+
|
28+
LL | for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) {
29+
| -^------------------- help: try adding parentheses: `(x, y @ Allosome::Y(_))`
30+
31+
error: unexpected `,` in pattern
32+
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:94:14
33+
|
34+
LL | let women, men: (Vec<Genome>, Vec<Genome>) = genomes.iter().cloned()
35+
| -----^---- help: try adding parentheses: `(women, men)`
36+
37+
error: aborting due to 6 previous errors
38+

0 commit comments

Comments
 (0)