From 1f04597c3ca3af45236ecb496bd30db5c57daae9 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 24 Feb 2018 20:41:16 -0800 Subject: [PATCH] in which parentheses are suggested for should-have-been-tuple-patterns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/libsyntax/parse/parser.rs | 44 ++++++++- ...-48492-tuple-destructure-missing-parens.rs | 97 +++++++++++++++++++ ...92-tuple-destructure-missing-parens.stderr | 38 ++++++++ 3 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.rs create mode 100644 src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index da2a22df997..847733e1e37 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3318,7 +3318,7 @@ impl<'a> Parser<'a> { mut attrs: ThinVec) -> PResult<'a, P> { // Parse: `for in ` - 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 @@ -3528,7 +3528,7 @@ impl<'a> Parser<'a> { fn parse_pats(&mut self) -> PResult<'a, Vec>> { 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, @@ -3554,7 +3554,12 @@ impl<'a> Parser<'a> { // Trailing commas are significant because (p) and (p,) are different patterns. fn parse_parenthesized_pat_list(&mut self) -> PResult<'a, (Vec>, Option, bool)> { self.expect(&token::OpenDelim(token::Paren))?; + let result = self.parse_pat_list()?; + self.expect(&token::CloseDelim(token::Paren))?; + Ok(result) + } + fn parse_pat_list(&mut self) -> PResult<'a, (Vec>, Option, bool)> { let mut fields = Vec::new(); let mut ddpos = None; let mut trailing_comma = false; @@ -3584,8 +3589,6 @@ impl<'a> Parser<'a> { self.span_err(self.prev_span, "trailing comma is not permitted after `..`"); } - self.expect(&token::CloseDelim(token::Paren))?; - Ok((fields, ddpos, trailing_comma)) } @@ -3767,6 +3770,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> { + 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; return 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_list() { + // 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)); + } + return Err(err); + } + Ok(pat) + } + /// Parse a pattern. pub fn parse_pat(&mut self) -> PResult<'a, P> { maybe_whole!(self, NtPat, |x| x); @@ -3969,7 +4003,7 @@ impl<'a> Parser<'a> { /// Parse a local variable declaration fn parse_local(&mut self, attrs: ThinVec) -> PResult<'a, P> { 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 `:` diff --git a/src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.rs b/src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.rs new file mode 100644 index 00000000000..7bdaaddad84 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.rs @@ -0,0 +1,97 @@ +// 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. + +#![allow(unused)] + +#[derive(Copy, Clone)] +enum Nucleotide { + Adenine, + Thymine, + Cytosine, + Guanine +} + +#[derive(Clone)] +struct Autosome; + +#[derive(Clone)] +enum Allosome { + X(Vec), + Y(Vec) +} + +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 { + let mut reading_frame = strand.windows(3); + // (missing parentheses in `while let` tuple pattern) + 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 { + let mut reading_frame = strand.windows(3); + let mut i = 0; + // (missing parentheses in `if let` tuple pattern) + if let b1, b2, b3 = reading_frame.next().unwrap() { + //~^ ERROR unexpected `,` in pattern + // ... + } + None +} + +fn is_thr(codon: (Nucleotide, Nucleotide, Nucleotide)) -> bool { + match codon { + // (missing parentheses in match arm tuple pattern) + Nucleotide::Adenine, Nucleotide::Cytosine, _ => true + //~^ ERROR unexpected `,` in pattern + _ => false + } +} + +fn analyze_female_sex_chromosomes(women: &[Genome]) { + // (missing parentheses in `for` tuple pattern) + for x, _barr_body in women.iter().map(|woman| woman.allosomes.clone()) { + //~^ ERROR unexpected `,` in pattern + // ... + } +} + +fn analyze_male_sex_chromosomes(men: &[Genome]) { + // (missing parentheses in pattern with `@` binding) + for x, y @ Allosome::Y(_) in men.iter().map(|man| man.allosomes.clone()) { + //~^ ERROR unexpected `,` in pattern + // ... + } +} + +fn main() { + let genomes = Vec::new(); + // (missing parentheses in `let` pattern) + let women, men: (Vec, Vec) = genomes.iter().cloned() + //~^ ERROR unexpected `,` in pattern + .partition(|g: &Genome| g.allosomes.0.is_x() && g.allosomes.1.is_x()); +} diff --git a/src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.stderr b/src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.stderr new file mode 100644 index 00000000000..db3f93af444 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-48492-tuple-destructure-missing-parens.stderr @@ -0,0 +1,38 @@ +error: unexpected `,` in pattern + --> $DIR/issue-48492-tuple-destructure-missing-parens.rs:48:17 + | +LL | 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:59:14 + | +LL | 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:69:28 + | +LL | Nucleotide::Adenine, Nucleotide::Cytosine, _ => true + | -------------------^------------------------ help: try adding parentheses: `(Nucleotide::Adenine, Nucleotide::Cytosine, _)` + +error: unexpected `,` in pattern + --> $DIR/issue-48492-tuple-destructure-missing-parens.rs:77:10 + | +LL | 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:85:10 + | +LL | 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:94:14 + | +LL | let women, men: (Vec, Vec) = genomes.iter().cloned() + | -----^---- help: try adding parentheses: `(women, men)` + +error: aborting due to 6 previous errors +