Skip to content

Commit

Permalink
in which parentheses are suggested for should-have-been-tuple-patterns
Browse files Browse the repository at this point in the history
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, 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 rust-lang#48492.
  • Loading branch information
zackmdavis committed Feb 28, 2018
1 parent affe297 commit ed6679c
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 3 deletions.
37 changes: 34 additions & 3 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3294,7 +3294,7 @@ impl<'a> Parser<'a> {
mut attrs: ThinVec<Attribute>) -> PResult<'a, P<Expr>> {
// Parse: `for <src_pat> in <src_expr> <src_loop_block>`

let pat = self.parse_pat()?;
let pat = self.parse_top_level_pat()?;
if !self.eat_keyword(keywords::In) {
let in_span = self.prev_span.between(self.span);
let mut err = self.sess.span_diagnostic
Expand Down Expand Up @@ -3466,7 +3466,7 @@ impl<'a> Parser<'a> {
fn parse_pats(&mut self) -> PResult<'a, Vec<P<Pat>>> {
let mut pats = Vec::new();
loop {
pats.push(self.parse_pat()?);
pats.push(self.parse_top_level_pat()?);

if self.token == token::OrOr {
let mut err = self.struct_span_err(self.span,
Expand Down Expand Up @@ -3690,6 +3690,37 @@ impl<'a> Parser<'a> {
}))
}

/// A wrapper around `parse_pat` with some special error handling for the
/// "top-level" patterns in a match arm, `for` loop, `let`, &c. (in contast
/// to subpatterns within such).
pub fn parse_top_level_pat(&mut self) -> PResult<'a, P<Pat>> {
let pat = self.parse_pat()?;
if self.token == token::Comma {
// An unexpected comma after a top-level pattern is a clue that the
// user (perhaps more accustomed to some other language) forgot the
// parentheses in what should have been a tuple pattern; emit a
// suggestion-enhanced error here rather than choking on the comma
// later.
let comma_span = self.span;
self.bump();
if let Err(mut err) = self.parse_pat_tuple_elements(false) {
// We didn't expect this to work anyway; we just wanted
// to advance to the end of the comma-sequence so we know
// the span to suggest parenthesizing
err.cancel();
}
let seq_span = pat.span.to(self.prev_span);
let mut err = self.struct_span_err(comma_span,
"unexpected `,` in pattern");
if let Ok(seq_snippet) = self.sess.codemap().span_to_snippet(seq_span) {
err.span_suggestion(seq_span, "try adding parentheses",
format!("({})", seq_snippet));
}
err.emit();
}
Ok(pat)
}

/// Parse a pattern.
pub fn parse_pat(&mut self) -> PResult<'a, P<Pat>> {
maybe_whole!(self, NtPat, |x| x);
Expand Down Expand Up @@ -3881,7 +3912,7 @@ impl<'a> Parser<'a> {
/// Parse a local variable declaration
fn parse_local(&mut self, attrs: ThinVec<Attribute>) -> PResult<'a, P<Local>> {
let lo = self.prev_span;
let pat = self.parse_pat()?;
let pat = self.parse_top_level_pat()?;

let (err, ty) = if self.eat(&token::Colon) {
// Save the state of the parser before parsing type normally, in case there is a `:`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused)]

#[derive(Copy, Clone)]
enum Nucleotide {
Adenine,
Thymine,
Cytosine,
Guanine
}

#[derive(Clone)]
struct Autosome;

#[derive(Clone)]
enum Allosome {
X(Vec<Nucleotide>),
Y(Vec<Nucleotide>)
}

impl Allosome {
fn is_x(&self) -> bool {
match *self {
Allosome::X(_) => true,
Allosome::Y(_) => false,
}
}
}

#[derive(Clone)]
struct Genome {
autosomes: [Autosome; 22],
allosomes: (Allosome, Allosome)
}

fn find_start_codon(strand: &[Nucleotide]) -> Option<usize> {
let mut reading_frame = strand.windows(3);
while let b1, b2, b3 = reading_frame.next().expect("there should be a start codon") {
//~^ ERROR unexpected `,` in pattern
// ...
}
None
}

fn find_thr(strand: &[Nucleotide]) -> Option<usize> {
let mut reading_frame = strand.windows(3);
let mut i = 0;
if let b1, b2, b3 = reading_frame.next().unwrap() {
//~^ ERROR unexpected `,` in pattern
match (b1, b2, b3) {
//~^ ERROR cannot find value `b2` in this scope
//~| ERROR cannot find value `b3` in this scope
Nucleotide::Adenine, Nucleotide::Cytosine, _ => {
//~^ ERROR unexpected `,` in pattern
return Some(i);
},
_ => {}
}
i += 1;
}
None
}

fn analyze_sex_chromosomes(women: &[Genome], men: &[Genome]) {
for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) {
//~^ ERROR unexpected `,` in pattern
// ...
}
for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) {
//~^ ERROR unexpected `,` in pattern
// ...
}
}

fn main() {
let genomes = Vec::new();
let women, men: (Vec<Genome>, Vec<Genome>) = genomes.iter().cloned()
//~^ ERROR unexpected `,` in pattern
.partition(|g: &Genome| g.allosomes.0.is_x() && g.allosomes.1.is_x());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:47:17
|
47 | while let b1, b2, b3 = reading_frame.next().expect("there should be a start codon") {
| --^------- help: try adding parentheses: `(b1, b2, b3)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:57:14
|
57 | if let b1, b2, b3 = reading_frame.next().unwrap() {
| --^------- help: try adding parentheses: `(b1, b2, b3)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:62:32
|
62 | Nucleotide::Adenine, Nucleotide::Cytosine, _ => {
| -------------------^------------------------ help: try adding parentheses: `(Nucleotide::Adenine, Nucleotide::Cytosine, _)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:74:10
|
74 | for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) {
| -^----------- help: try adding parentheses: `(x, _barr_body)`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:78:10
|
78 | for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) {
| -^------------------- help: try adding parentheses: `(x, y @ Allosome::Y(_))`

error: unexpected `,` in pattern
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:86:14
|
86 | let women, men: (Vec<Genome>, Vec<Genome>) = genomes.iter().cloned()
| -----^---- help: try adding parentheses: `(women, men)`

error[E0425]: cannot find value `b2` in this scope
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:59:20
|
59 | match (b1, b2, b3) {
| ^^ did you mean `b1`?

error[E0425]: cannot find value `b3` in this scope
--> $DIR/issue-48492-tuple-destructure-missing-parens.rs:59:24
|
59 | match (b1, b2, b3) {
| ^^ did you mean `b1`?

error: aborting due to 8 previous errors

0 comments on commit ed6679c

Please sign in to comment.