From 1f04597c3ca3af45236ecb496bd30db5c57daae9 Mon Sep 17 00:00:00 2001
From: "Zack M. Davis" <code@zackmdavis.net>
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<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
@@ -3528,7 +3528,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,
@@ -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<P<Pat>>, Option<usize>, 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<P<Pat>>, Option<usize>, 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<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; 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<Pat>> {
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<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 `:`
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 <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);
+ // (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<usize> {
+ 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<Genome>, Vec<Genome>) = 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<Genome>, Vec<Genome>) = genomes.iter().cloned()
+ | -----^---- help: try adding parentheses: `(women, men)`
+
+error: aborting due to 6 previous errors
+