Skip to content

Commit

Permalink
Detect blocks that could be struct expr bodies
Browse files Browse the repository at this point in the history
This approach lives exclusively in the parser, so struct expr bodies
that are syntactically correct on their own but are otherwise incorrect
will still emit confusing errors, like in the following case:

```rust
fn foo() -> Foo {
    bar: Vec::new()
}
```

```
error[E0425]: cannot find value `bar` in this scope
 --> src/file.rs:5:5
  |
5 |     bar: Vec::new()
  |     ^^^ expecting a type here because of type ascription

error[E0214]: parenthesized type parameters may only be used with a `Fn` trait
 --> src/file.rs:5:15
  |
5 |     bar: Vec::new()
  |               ^^^^^ only `Fn` traits may use parentheses

error[E0107]: wrong number of type arguments: expected 1, found 0
 --> src/file.rs:5:10
  |
5 |     bar: Vec::new()
  |          ^^^^^^^^^^ expected 1 type argument
  ```

If that field had a trailing comma, that would be a parse error and it
would trigger the new, more targetted, error:

```
error: struct literal body without path
 --> file.rs:4:17
  |
4 |   fn foo() -> Foo {
  |  _________________^
5 | |     bar: Vec::new(),
6 | | }
  | |_^
  |
help: you might have forgotten to add the struct literal inside the block
  |
4 | fn foo() -> Foo { Path {
5 |     bar: Vec::new(),
6 | } }
  |
```

Partially address last part of rust-lang#34255.
  • Loading branch information
estebank committed Oct 7, 2020
1 parent deec530 commit e5f83bc
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 17 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_data_structures::map_in_place::MapInPlace;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::{struct_span_err, Applicability, PResult};
use rustc_feature::Features;
use rustc_parse::parser::Parser;
use rustc_parse::parser::{AttemptLocalParseRecovery, Parser};
use rustc_parse::validate_attr;
use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS;
use rustc_session::lint::BuiltinLintDiagnostics;
Expand Down Expand Up @@ -921,7 +921,7 @@ pub fn parse_ast_fragment<'a>(
let mut stmts = SmallVec::new();
// Won't make progress on a `}`.
while this.token != token::Eof && this.token != token::CloseDelim(token::Brace) {
if let Some(stmt) = this.parse_full_stmt()? {
if let Some(stmt) = this.parse_full_stmt(AttemptLocalParseRecovery::Yes)? {
stmts.push(stmt);
}
}
Expand Down
87 changes: 85 additions & 2 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use rustc_ast::ptr::P;
use rustc_ast::token::{self, Lit, LitKind, TokenKind};
use rustc_ast::util::parser::AssocOp;
use rustc_ast::{
self as ast, AngleBracketedArgs, AttrVec, BinOpKind, BindingMode, BlockCheckMode, Expr,
ExprKind, Item, ItemKind, Mutability, Param, Pat, PatKind, PathSegment, QSelf, Ty, TyKind,
self as ast, AngleBracketedArgs, AttrVec, BinOpKind, BindingMode, Block, BlockCheckMode, Expr,
ExprKind, Item, ItemKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QSelf, Ty,
TyKind,
};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -119,6 +120,28 @@ crate enum ConsumeClosingDelim {
No,
}

#[derive(Clone, Copy)]
pub enum AttemptLocalParseRecovery {
Yes,
No,
}

impl AttemptLocalParseRecovery {
pub fn yes(&self) -> bool {
match self {
AttemptLocalParseRecovery::Yes => true,
AttemptLocalParseRecovery::No => false,
}
}

pub fn no(&self) -> bool {
match self {
AttemptLocalParseRecovery::Yes => false,
AttemptLocalParseRecovery::No => true,
}
}
}

impl<'a> Parser<'a> {
pub(super) fn span_fatal_err<S: Into<MultiSpan>>(
&self,
Expand Down Expand Up @@ -321,6 +344,66 @@ impl<'a> Parser<'a> {
}
}

pub fn maybe_suggest_struct_literal(
&mut self,
lo: Span,
s: BlockCheckMode,
) -> Option<PResult<'a, P<Block>>> {
if self.token.is_ident() && self.look_ahead(1, |t| t == &token::Colon) {
// We might be having a struct literal where people forgot to include the path:
// fn foo() -> Foo {
// field: value,
// }
let mut snapshot = self.clone();
let path =
Path { segments: vec![], span: self.prev_token.span.shrink_to_lo(), tokens: None };
let struct_expr = snapshot.parse_struct_expr(path, AttrVec::new(), false);
let block_tail = self.parse_block_tail(lo, s, AttemptLocalParseRecovery::No);
return Some(match (struct_expr, block_tail) {
(Ok(expr), Err(mut err)) => {
// We have encountered the following:
// fn foo() -> Foo {
// field: value,
// }
// Suggest:
// fn foo() -> Foo { Path {
// field: value,
// } }
err.delay_as_bug();
self.struct_span_err(expr.span, "struct literal body without path")
.multipart_suggestion(
"you might have forgotten to add the struct literal inside the block",
vec![
(expr.span.shrink_to_lo(), "{ SomeStruct ".to_string()),
(expr.span.shrink_to_hi(), " }".to_string()),
],
Applicability::MaybeIncorrect,
)
.emit();
*self = snapshot;
Ok(self.mk_block(
vec![self.mk_stmt_err(expr.span)],
s,
lo.to(self.prev_token.span),
))
}
(Err(mut err), Ok(tail)) => {
// We have a block tail that contains a somehow valid type ascription expr.
err.cancel();
Ok(tail)
}
(Err(mut snapshot_err), Err(err)) => {
// We don't know what went wrong, emit the normal error.
snapshot_err.cancel();
self.consume_block(token::Brace, ConsumeClosingDelim::Yes);
Err(err)
}
(Ok(_), Ok(tail)) => Ok(tail),
});
}
None
}

pub fn maybe_annotate_with_ascription(
&mut self,
err: &mut DiagnosticBuilder<'_>,
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,9 +2015,12 @@ impl<'a> Parser<'a> {
) -> Option<PResult<'a, P<Expr>>> {
let struct_allowed = !self.restrictions.contains(Restrictions::NO_STRUCT_LITERAL);
if struct_allowed || self.is_certainly_not_a_block() {
// This is a struct literal, but we don't can't accept them here.
let expr = self.parse_struct_expr(path.clone(), attrs.clone());
if let Err(err) = self.expect(&token::OpenDelim(token::Brace)) {
return Some(Err(err));
}
let expr = self.parse_struct_expr(path.clone(), attrs.clone(), true);
if let (Ok(expr), false) = (&expr, struct_allowed) {
// This is a struct literal, but we don't can't accept them here.
self.error_struct_lit_not_allowed_here(path.span, expr.span);
}
return Some(expr);
Expand All @@ -2035,12 +2038,13 @@ impl<'a> Parser<'a> {
.emit();
}

/// Precondition: already parsed the '{'.
pub(super) fn parse_struct_expr(
&mut self,
pth: ast::Path,
mut attrs: AttrVec,
recover: bool,
) -> PResult<'a, P<Expr>> {
self.bump();
let mut fields = Vec::new();
let mut base = None;
let mut recover_async = false;
Expand All @@ -2059,10 +2063,11 @@ impl<'a> Parser<'a> {
let exp_span = self.prev_token.span;
match self.parse_expr() {
Ok(e) => base = Some(e),
Err(mut e) => {
Err(mut e) if recover => {
e.emit();
self.recover_stmt();
}
Err(e) => return Err(e),
}
self.recover_struct_comma_after_dotdot(exp_span);
break;
Expand Down Expand Up @@ -2114,6 +2119,9 @@ impl<'a> Parser<'a> {
);
}
}
if !recover {
return Err(e);
}
e.emit();
self.recover_stmt_(SemiColonMode::Comma, BlockMode::Ignore);
self.eat(&token::Comma);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod stmt;
mod ty;

use crate::lexer::UnmatchedBrace;
pub use diagnostics::AttemptLocalParseRecovery;
use diagnostics::Error;
pub use path::PathStyle;

Expand Down
36 changes: 27 additions & 9 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::attr::DEFAULT_INNER_ATTR_FORBIDDEN;
use super::diagnostics::Error;
use super::diagnostics::{AttemptLocalParseRecovery, Error};
use super::expr::LhsExpr;
use super::pat::GateOr;
use super::path::PathStyle;
Expand Down Expand Up @@ -79,8 +79,8 @@ impl<'a> Parser<'a> {
return self.parse_stmt_mac(lo, attrs.into(), path);
}

let expr = if self.check(&token::OpenDelim(token::Brace)) {
self.parse_struct_expr(path, AttrVec::new())?
let expr = if self.eat(&token::OpenDelim(token::Brace)) {
self.parse_struct_expr(path, AttrVec::new(), true)?
} else {
let hi = self.prev_token.span;
self.mk_expr(lo.to(hi), ExprKind::Path(None, path), AttrVec::new())
Expand Down Expand Up @@ -321,25 +321,37 @@ impl<'a> Parser<'a> {
return self.error_block_no_opening_brace();
}

Ok((self.parse_inner_attributes()?, self.parse_block_tail(lo, blk_mode)?))
let attrs = self.parse_inner_attributes()?;
let tail = if let Some(tail) = self.maybe_suggest_struct_literal(lo, blk_mode) {
tail?
} else {
self.parse_block_tail(lo, blk_mode, AttemptLocalParseRecovery::Yes)?
};
Ok((attrs, tail))
}

/// Parses the rest of a block expression or function body.
/// Precondition: already parsed the '{'.
fn parse_block_tail(&mut self, lo: Span, s: BlockCheckMode) -> PResult<'a, P<Block>> {
crate fn parse_block_tail(
&mut self,
lo: Span,
s: BlockCheckMode,
recover: AttemptLocalParseRecovery,
) -> PResult<'a, P<Block>> {
let mut stmts = vec![];
while !self.eat(&token::CloseDelim(token::Brace)) {
if self.token == token::Eof {
break;
}
let stmt = match self.parse_full_stmt() {
Err(mut err) => {
let stmt = match self.parse_full_stmt(recover) {
Err(mut err) if recover.yes() => {
self.maybe_annotate_with_ascription(&mut err, false);
err.emit();
self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Ignore);
Some(self.mk_stmt_err(self.token.span))
}
Ok(stmt) => stmt,
Err(err) => return Err(err),
};
if let Some(stmt) = stmt {
stmts.push(stmt);
Expand All @@ -352,7 +364,10 @@ impl<'a> Parser<'a> {
}

/// Parses a statement, including the trailing semicolon.
pub fn parse_full_stmt(&mut self) -> PResult<'a, Option<Stmt>> {
pub fn parse_full_stmt(
&mut self,
recover: AttemptLocalParseRecovery,
) -> PResult<'a, Option<Stmt>> {
// Skip looking for a trailing semicolon when we have an interpolated statement.
maybe_whole!(self, NtStmt, |x| Some(x));

Expand Down Expand Up @@ -391,6 +406,9 @@ impl<'a> Parser<'a> {
if let Err(mut e) =
self.check_mistyped_turbofish_with_multiple_type_params(e, expr)
{
if recover.no() {
return Err(e);
}
e.emit();
self.recover_stmt();
}
Expand Down Expand Up @@ -432,7 +450,7 @@ impl<'a> Parser<'a> {
Stmt { id: DUMMY_NODE_ID, kind, span, tokens: None }
}

fn mk_stmt_err(&self, span: Span) -> Stmt {
pub(super) fn mk_stmt_err(&self, span: Span) -> Stmt {
self.mk_stmt(span, StmtKind::Expr(self.mk_expr_err(span)))
}

Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/parser/bare-struct-body.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
struct Foo {
val: (),
}

fn foo() -> Foo { //~ ERROR struct literal body without path
val: (),
}

fn main() {
let x = foo();
x.val == 42; //~ ERROR mismatched types
let x = { //~ ERROR struct literal body without path
val: (),
};
}
41 changes: 41 additions & 0 deletions src/test/ui/parser/bare-struct-body.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
error: struct literal body without path
--> $DIR/bare-struct-body.rs:5:17
|
LL | fn foo() -> Foo {
| _________________^
LL | | val: (),
LL | | }
| |_^
|
help: you might have forgotten to add the struct literal inside the block
|
LL | fn foo() -> Foo { SomeStruct {
LL | val: (),
LL | } }
|

error: struct literal body without path
--> $DIR/bare-struct-body.rs:12:13
|
LL | let x = {
| _____________^
LL | | val: (),
LL | | };
| |_____^
|
help: you might have forgotten to add the struct literal inside the block
|
LL | let x = { SomeStruct {
LL | val: (),
LL | } };
|

error[E0308]: mismatched types
--> $DIR/bare-struct-body.rs:11:14
|
LL | x.val == 42;
| ^^ expected `()`, found integer

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.

0 comments on commit e5f83bc

Please sign in to comment.