Skip to content

Improve diagnostics for functions in struct definitions #76808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 40 additions & 12 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1124,11 +1124,11 @@ impl<'a> Parser<'a> {
if !this.recover_nested_adt_item(kw::Enum)? {
return Ok((None, TrailingToken::None));
}
let ident = this.parse_ident()?;
let ident = this.parse_field_ident("enum", vlo)?;

let struct_def = if this.check(&token::OpenDelim(token::Brace)) {
// Parse a struct variant.
let (fields, recovered) = this.parse_record_struct_body()?;
let (fields, recovered) = this.parse_record_struct_body("struct")?;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this can give some slightly ankward results:

enum Foo {
    Variant {
        fn bar() {}
    }
}
error: functions are not allowed in struct definitions
 --> test.rs:5:9
  |
5 |         fn bar() {}
  |         ^^^^^^^^^^^
  |
  = help: unlike in C++, Java, and C#, functions are declared in `impl` blocks
  = help: see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information

Doesn't seem very important to me however

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see. If it comes up, a fairly easy fix might be to add a struct flag that is true in an actual struct and false in a enum variant.

VariantData::Struct(fields, recovered)
} else if this.check(&token::OpenDelim(token::Paren)) {
VariantData::Tuple(this.parse_tuple_struct_body()?, DUMMY_NODE_ID)
Expand Down Expand Up @@ -1182,15 +1182,15 @@ impl<'a> Parser<'a> {
VariantData::Unit(DUMMY_NODE_ID)
} else {
// If we see: `struct Foo<T> where T: Copy { ... }`
let (fields, recovered) = self.parse_record_struct_body()?;
let (fields, recovered) = self.parse_record_struct_body("struct")?;
VariantData::Struct(fields, recovered)
}
// No `where` so: `struct Foo<T>;`
} else if self.eat(&token::Semi) {
VariantData::Unit(DUMMY_NODE_ID)
// Record-style struct definition
} else if self.token == token::OpenDelim(token::Brace) {
let (fields, recovered) = self.parse_record_struct_body()?;
let (fields, recovered) = self.parse_record_struct_body("struct")?;
VariantData::Struct(fields, recovered)
// Tuple-style struct definition with optional where-clause.
} else if self.token == token::OpenDelim(token::Paren) {
Expand Down Expand Up @@ -1220,10 +1220,10 @@ impl<'a> Parser<'a> {

let vdata = if self.token.is_keyword(kw::Where) {
generics.where_clause = self.parse_where_clause()?;
let (fields, recovered) = self.parse_record_struct_body()?;
let (fields, recovered) = self.parse_record_struct_body("union")?;
VariantData::Struct(fields, recovered)
} else if self.token == token::OpenDelim(token::Brace) {
let (fields, recovered) = self.parse_record_struct_body()?;
let (fields, recovered) = self.parse_record_struct_body("union")?;
VariantData::Struct(fields, recovered)
} else {
let token_str = super::token_descr(&self.token);
Expand All @@ -1236,12 +1236,15 @@ impl<'a> Parser<'a> {
Ok((class_name, ItemKind::Union(vdata, generics)))
}

fn parse_record_struct_body(&mut self) -> PResult<'a, (Vec<FieldDef>, /* recovered */ bool)> {
fn parse_record_struct_body(
&mut self,
adt_ty: &str,
) -> PResult<'a, (Vec<FieldDef>, /* recovered */ bool)> {
let mut fields = Vec::new();
let mut recovered = false;
if self.eat(&token::OpenDelim(token::Brace)) {
while self.token != token::CloseDelim(token::Brace) {
let field = self.parse_field_def().map_err(|e| {
let field = self.parse_field_def(adt_ty).map_err(|e| {
self.consume_block(token::Brace, ConsumeClosingDelim::No);
recovered = true;
e
Expand Down Expand Up @@ -1294,24 +1297,25 @@ impl<'a> Parser<'a> {
}

/// Parses an element of a struct declaration.
fn parse_field_def(&mut self) -> PResult<'a, FieldDef> {
fn parse_field_def(&mut self, adt_ty: &str) -> PResult<'a, FieldDef> {
let attrs = self.parse_outer_attributes()?;
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;
let vis = this.parse_visibility(FollowedByType::No)?;
Ok((this.parse_single_struct_field(lo, vis, attrs)?, TrailingToken::None))
Ok((this.parse_single_struct_field(adt_ty, lo, vis, attrs)?, TrailingToken::None))
})
}

/// Parses a structure field declaration.
fn parse_single_struct_field(
&mut self,
adt_ty: &str,
lo: Span,
vis: Visibility,
attrs: Vec<Attribute>,
) -> PResult<'a, FieldDef> {
let mut seen_comma: bool = false;
let a_var = self.parse_name_and_ty(lo, vis, attrs)?;
let a_var = self.parse_name_and_ty(adt_ty, lo, vis, attrs)?;
if self.token == token::Comma {
seen_comma = true;
}
Expand Down Expand Up @@ -1398,11 +1402,12 @@ impl<'a> Parser<'a> {
/// Parses a structure field.
fn parse_name_and_ty(
&mut self,
adt_ty: &str,
lo: Span,
vis: Visibility,
attrs: Vec<Attribute>,
) -> PResult<'a, FieldDef> {
let name = self.parse_ident_common(false)?;
let name = self.parse_field_ident(adt_ty, lo)?;
self.expect(&token::Colon)?;
let ty = self.parse_ty()?;
Ok(FieldDef {
Expand All @@ -1416,6 +1421,29 @@ impl<'a> Parser<'a> {
})
}

/// Parses a field identifier. Specialized version of `parse_ident_common`
/// for better diagnostics and suggestions.
fn parse_field_ident(&mut self, adt_ty: &str, lo: Span) -> PResult<'a, Ident> {
let (ident, is_raw) = self.ident_or_err()?;
if !is_raw && ident.is_reserved() {
let err = if self.check_fn_front_matter(false) {
let _ = self.parse_fn(&mut Vec::new(), |_| true, lo);
let mut err = self.struct_span_err(
lo.to(self.prev_token.span),
&format!("functions are not allowed in {} definitions", adt_ty),
);
err.help("unlike in C++, Java, and C#, functions are declared in `impl` blocks");
err.help("see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information");
err
} else {
self.expected_ident_found()
};
return Err(err);
}
self.bump();
Ok(ident)
}

/// Parses a declarative macro 2.0 definition.
/// The `macro` keyword has already been parsed.
/// ```
Expand Down
36 changes: 18 additions & 18 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,27 +522,27 @@ impl<'a> Parser<'a> {
self.parse_ident_common(true)
}

fn ident_or_err(&mut self) -> PResult<'a, (Ident, /* is_raw */ bool)> {
self.token.ident().ok_or_else(|| match self.prev_token.kind {
TokenKind::DocComment(..) => {
self.span_fatal_err(self.prev_token.span, Error::UselessDocComment)
}
_ => self.expected_ident_found(),
})
}

fn parse_ident_common(&mut self, recover: bool) -> PResult<'a, Ident> {
match self.token.ident() {
Some((ident, is_raw)) => {
if !is_raw && ident.is_reserved() {
let mut err = self.expected_ident_found();
if recover {
err.emit();
} else {
return Err(err);
}
}
self.bump();
Ok(ident)
let (ident, is_raw) = self.ident_or_err()?;
if !is_raw && ident.is_reserved() {
let mut err = self.expected_ident_found();
if recover {
err.emit();
} else {
return Err(err);
}
_ => Err(match self.prev_token.kind {
TokenKind::DocComment(..) => {
self.span_fatal_err(self.prev_token.span, Error::UselessDocComment)
}
_ => self.expected_ident_found(),
}),
}
self.bump();
Ok(ident)
}

/// Checks if the next token is `tok`, and returns `true` if so.
Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/structs/struct-fn-in-definition.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// It might be intuitive for a user coming from languages like Java
// to declare a method directly in a struct's definition. Make sure
// rustc can give a helpful suggestion.
// Suggested in issue #76421

struct S {
field: usize,

fn foo() {}
//~^ ERROR functions are not allowed in struct definitions
//~| HELP unlike in C++, Java, and C#, functions are declared in `impl` blocks
//~| HELP see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information
}

union U {
variant: usize,

fn foo() {}
//~^ ERROR functions are not allowed in union definitions
//~| HELP unlike in C++, Java, and C#, functions are declared in `impl` blocks
//~| HELP see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information
}

enum E {
Variant,

fn foo() {}
//~^ ERROR functions are not allowed in enum definitions
//~| HELP unlike in C++, Java, and C#, functions are declared in `impl` blocks
//~| HELP see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information
}

fn main() {}
29 changes: 29 additions & 0 deletions src/test/ui/structs/struct-fn-in-definition.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error: functions are not allowed in struct definitions
--> $DIR/struct-fn-in-definition.rs:9:5
|
LL | fn foo() {}
| ^^^^^^^^^^^
|
= help: unlike in C++, Java, and C#, functions are declared in `impl` blocks
= help: see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information

error: functions are not allowed in union definitions
--> $DIR/struct-fn-in-definition.rs:18:5
|
LL | fn foo() {}
| ^^^^^^^^^^^
|
= help: unlike in C++, Java, and C#, functions are declared in `impl` blocks
= help: see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information

error: functions are not allowed in enum definitions
--> $DIR/struct-fn-in-definition.rs:27:5
|
LL | fn foo() {}
| ^^^^^^^^^^^
|
= help: unlike in C++, Java, and C#, functions are declared in `impl` blocks
= help: see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information

error: aborting due to 3 previous errors