Skip to content

Commit

Permalink
remove duplicated code and simplify logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@estebank
estebank committed Apr 19, 2019
1 parent f1be8d1 commit 4c4ca60
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 94 deletions.
93 changes: 34 additions & 59 deletions src/librustc_resolve/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(..), _) => {
Expand Down Expand Up @@ -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);
Expand Down
61 changes: 31 additions & 30 deletions src/libsyntax/parse/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Attribute>,
) -> Option<PResult<'a, P<Expr>>> {
// We don't want to assume it's a struct when encountering `{ <ident>: <ident> }` 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
}
Expand Down
15 changes: 14 additions & 1 deletion src/test/ui/struct-literal-variant-in-if.rs
View file
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
70 changes: 66 additions & 4 deletions src/test/ui/struct-literal-variant-in-if.stderr
View file
Original file line number Diff line number Diff line change
@@ -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: `<expr>: <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`
Expand All @@ -19,15 +81,15 @@ 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 ()
|
= 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`.

0 comments on commit 4c4ca60

Please sign in to comment.