From 4c4ca60edd9775873e555dbb5928b000bd734403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 18 Apr 2019 11:35:11 -0700 Subject: [PATCH] remove duplicated code and simplify logic --- src/librustc_resolve/diagnostics.rs | 93 +++++++------------ src/libsyntax/parse/parser.rs | 61 ++++++------ src/test/ui/struct-literal-variant-in-if.rs | 15 ++- .../ui/struct-literal-variant-in-if.stderr | 70 +++++++++++++- 4 files changed, 145 insertions(+), 94 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index b7deb54688215..c89c222ad57fc 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -328,7 +328,38 @@ impl<'a> Resolver<'a> { _ => false, }; - let (followed_by_brace, closing_brace) = self.followed_by_brace(span); + let mut bad_struct_syntax_suggestion = || { + let (followed_by_brace, closing_brace) = self.followed_by_brace(span); + let mut suggested = false; + match source { + PathSource::Expr(Some(parent)) => { + suggested = path_sep(err, &parent); + } + PathSource::Expr(None) if followed_by_brace == true => { + if let Some((sp, snippet)) = closing_brace { + err.span_suggestion( + sp, + "surround the struct literal with parenthesis", + format!("({})", snippet), + Applicability::MaybeIncorrect, + ); + } else { + err.span_label( + span, // Note the parenthesis surrounding the suggestion below + format!("did you mean `({} {{ /* fields */ }})`?", path_str), + ); + } + suggested = true; + }, + _ => {} + } + if !suggested { + err.span_label( + span, + format!("did you mean `{} {{ /* fields */ }}`?", path_str), + ); + } + }; match (def, source) { (Def::Macro(..), _) => { @@ -383,69 +414,13 @@ impl<'a> Resolver<'a> { ); } } else { - match source { - PathSource::Expr(Some(parent)) => if !path_sep(err, &parent) { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", path_str), - ); - } - PathSource::Expr(None) if followed_by_brace == true => { - if let Some((sp, snippet)) = closing_brace { - err.span_suggestion( - sp, - "surround the struct literal with parenthesis", - format!("({})", snippet), - Applicability::MaybeIncorrect, - ); - } else { - err.span_label( - span, - format!("did you mean `({} {{ /* fields */ }})`?", path_str), - ); - } - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", path_str), - ); - }, - } + bad_struct_syntax_suggestion(); } } (Def::Union(..), _) | (Def::Variant(..), _) | (Def::Ctor(_, _, CtorKind::Fictive), _) if ns == ValueNS => { - match source { - PathSource::Expr(Some(parent)) => if !path_sep(err, &parent) { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", path_str), - ); - } - PathSource::Expr(None) if followed_by_brace == true => { - if let Some((sp, snippet)) = closing_brace { - err.span_suggestion( - sp, - "surround the struct literal with parenthesis", - format!("({})", snippet), - Applicability::MaybeIncorrect, - ); - } else { - err.span_label( - span, - format!("did you mean `({} {{ /* fields */ }})`?", path_str), - ); - } - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", path_str), - ); - }, - } + bad_struct_syntax_suggestion(); } (Def::SelfTy(..), _) if ns == ValueNS => { err.span_label(span, fallback_label); diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a82b1e11a5227..a2a2c4637c571 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2856,7 +2856,7 @@ impl<'a> Parser<'a> { hi = self.prev_span; ex = ExprKind::Mac(respan(lo.to(hi), Mac_ { path, tts, delim })); } else if self.check(&token::OpenDelim(token::Brace)) { - if let Some(expr) = self.should_parse_struct_expr(lo, &path, &attrs) { + if let Some(expr) = self.maybe_parse_struct_expr(lo, &path, &attrs) { return expr; } else { hi = path.span; @@ -2904,47 +2904,48 @@ impl<'a> Parser<'a> { self.maybe_recover_from_bad_qpath(expr, true) } - fn should_parse_struct_expr( + fn maybe_parse_struct_expr( &mut self, lo: Span, path: &ast::Path, attrs: &ThinVec, ) -> Option>> { + // We don't want to assume it's a struct when encountering `{ : }` because + // it could be type ascription, like in `{ ident: u32 }`. + let isnt_ascription = self.look_ahead(1, |t| t.is_ident()) && + self.look_ahead(2, |t| *t == token::Colon) && ( + (self.look_ahead(3, |t| t.is_ident()) && + self.look_ahead(4, |t| *t == token::Comma)) || + self.look_ahead(3, |t| t.is_lit()) || + self.look_ahead(3, |t| *t == token::BinOp(token::Minus)) && + self.look_ahead(4, |t| t.is_lit()) + ); let could_be_struct = self.look_ahead(1, |t| t.is_ident()) && ( - self.look_ahead(2, |t| *t == token::Colon) + self.look_ahead(2, |t| *t == token::Colon) && isnt_ascription || self.look_ahead(2, |t| *t == token::Comma) // We could also check for `token::CloseDelim(token::Brace)`, but that would // have false positives in the case of `if x == y { z } { a }`. ); - let mut bad_struct = false; - let mut parse_struct = !self.restrictions.contains(Restrictions::NO_STRUCT_LITERAL); - if self.restrictions.contains(Restrictions::NO_STRUCT_LITERAL) && could_be_struct { + let bad_struct = self.restrictions.contains(Restrictions::NO_STRUCT_LITERAL); + if !bad_struct || could_be_struct { // This is a struct literal, but we don't can't accept them here - bad_struct = true; - parse_struct = true; - } - if parse_struct { - match self.parse_struct_expr(lo, path.clone(), attrs.clone()) { - Err(err) => return Some(Err(err)), - Ok(expr) => { - if bad_struct { - let mut err = self.diagnostic().struct_span_err( - expr.span, - "struct literals are not allowed here", - ); - err.multipart_suggestion( - "surround the struct literal with parenthesis", - vec![ - (lo.shrink_to_lo(), "(".to_string()), - (expr.span.shrink_to_hi(), ")".to_string()), - ], - Applicability::MachineApplicable, - ); - err.emit(); - } - return Some(Ok(expr)); - } + let expr = self.parse_struct_expr(lo, path.clone(), attrs.clone()); + if let (Ok(expr), true) = (&expr, bad_struct) { + let mut err = self.diagnostic().struct_span_err( + expr.span, + "struct literals are not allowed here", + ); + err.multipart_suggestion( + "surround the struct literal with parenthesis", + vec![ + (lo.shrink_to_lo(), "(".to_string()), + (expr.span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MachineApplicable, + ); + err.emit(); } + return Some(expr); } None } diff --git a/src/test/ui/struct-literal-variant-in-if.rs b/src/test/ui/struct-literal-variant-in-if.rs index 2d87c4ca73d01..8f2d50586c055 100644 --- a/src/test/ui/struct-literal-variant-in-if.rs +++ b/src/test/ui/struct-literal-variant-in-if.rs @@ -1,12 +1,25 @@ #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] enum E { - V { field: bool } + V { field: bool }, + I { field1: bool, field2: usize }, + J { field: isize }, + K { field: &'static str}, } fn test_E(x: E) { let field = true; if x == E::V { field } {} //~^ ERROR expected value, found struct variant `E::V` //~| ERROR mismatched types + if x == E::I { field1: true, field2: 42 } {} + //~^ ERROR struct literals are not allowed here + if x == E::V { field: false } {} + //~^ ERROR expected identifier, found keyword `false` + //~| ERROR expected type, found keyword `false` + //~| ERROR expected value, found struct variant `E::V` + if x == E::J { field: -42 } {} + //~^ ERROR struct literals are not allowed here + if x == E::K { field: "" } {} + //~^ ERROR struct literals are not allowed here let y: usize = (); //~^ ERROR mismatched types } diff --git a/src/test/ui/struct-literal-variant-in-if.stderr b/src/test/ui/struct-literal-variant-in-if.stderr index e38eb0d61e060..0af0c6aefaf40 100644 --- a/src/test/ui/struct-literal-variant-in-if.stderr +++ b/src/test/ui/struct-literal-variant-in-if.stderr @@ -1,13 +1,75 @@ +error: struct literals are not allowed here + --> $DIR/struct-literal-variant-in-if.rs:13:13 + | +LL | if x == E::I { field1: true, field2: 42 } {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: surround the struct literal with parenthesis + | +LL | if x == (E::I { field1: true, field2: 42 }) {} + | ^ ^ + +error: expected identifier, found keyword `false` + --> $DIR/struct-literal-variant-in-if.rs:15:27 + | +LL | if x == E::V { field: false } {} + | ^^^^^ expected identifier, found keyword +help: you can escape reserved keywords to use them as identifiers + | +LL | if x == E::V { field: r#false } {} + | ^^^^^^^ + +error: expected type, found keyword `false` + --> $DIR/struct-literal-variant-in-if.rs:15:27 + | +LL | if x == E::V { field: false } {} + | ^^^^^ expecting a type here because of type ascription + | + = note: type ascription is a nightly-only feature that lets you annotate an expression with a type: `: ` +note: this expression expects an ascribed type after the colon + --> $DIR/struct-literal-variant-in-if.rs:15:20 + | +LL | if x == E::V { field: false } {} + | ^^^^^ + = help: this might be indicative of a syntax error elsewhere + +error: struct literals are not allowed here + --> $DIR/struct-literal-variant-in-if.rs:19:13 + | +LL | if x == E::J { field: -42 } {} + | ^^^^^^^^^^^^^^^^^^^ +help: surround the struct literal with parenthesis + | +LL | if x == (E::J { field: -42 }) {} + | ^ ^ + +error: struct literals are not allowed here + --> $DIR/struct-literal-variant-in-if.rs:21:13 + | +LL | if x == E::K { field: "" } {} + | ^^^^^^^^^^^^^^^^^^ +help: surround the struct literal with parenthesis + | +LL | if x == (E::K { field: "" }) {} + | ^ ^ + error[E0423]: expected value, found struct variant `E::V` - --> $DIR/struct-literal-variant-in-if.rs:7:13 + --> $DIR/struct-literal-variant-in-if.rs:10:13 | LL | if x == E::V { field } {} | ^^^^---------- | | | help: surround the struct literal with parenthesis: `(E::V { field })` +error[E0423]: expected value, found struct variant `E::V` + --> $DIR/struct-literal-variant-in-if.rs:15:13 + | +LL | if x == E::V { field: false } {} + | ^^^^----------------- + | | + | help: surround the struct literal with parenthesis: `(E::V { field: false })` + error[E0308]: mismatched types - --> $DIR/struct-literal-variant-in-if.rs:7:20 + --> $DIR/struct-literal-variant-in-if.rs:10:20 | LL | fn test_E(x: E) { | - help: try adding a return type: `-> bool` @@ -19,7 +81,7 @@ LL | if x == E::V { field } {} found type `bool` error[E0308]: mismatched types - --> $DIR/struct-literal-variant-in-if.rs:10:20 + --> $DIR/struct-literal-variant-in-if.rs:23:20 | LL | let y: usize = (); | ^^ expected usize, found () @@ -27,7 +89,7 @@ LL | let y: usize = (); = note: expected type `usize` found type `()` -error: aborting due to 3 previous errors +error: aborting due to 9 previous errors Some errors occurred: E0308, E0423. For more information about an error, try `rustc --explain E0308`.