Skip to content
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

Give a better error message for functions in struct { ... } #76421

Closed
jyn514 opened this issue Sep 6, 2020 · 5 comments
Closed

Give a better error message for functions in struct { ... } #76421

jyn514 opened this issue Sep 6, 2020 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST C-feature-request Category: A feature request, i.e: not implemented / a PR. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Sep 6, 2020

Thought of this while reading https://fasterthanli.me/articles/i-am-a-java-csharp-c-or-cplusplus-dev-time-to-do-some-rust (thank you @fasterthanlime !). Currently putting a function in a struct definition is a syntax error without much hint of what went wrong (playground):

struct S {
    field: usize,
    
    fn do_something() {
    }
}
error: expected identifier, found keyword `fn`
 --> src/lib.rs:4:5
  |
4 |     fn do_something() {
  |     ^^ expected identifier, found keyword

error: expected `:`, found `do_something`
 --> src/lib.rs:4:8
  |
4 |     fn do_something() {
  |        ^^^^^^^^^^^^ expected `:`

error: aborting due to 2 previous errors

Instead it should say something like

error: functions are not allowed in struct definitions
 --> src/lib.rs:4:5
  |
4 |     fn do_something() {
  |     ^^^^^^^^^^^^^^^^^^^
  = 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
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints C-feature-request Category: A feature request, i.e: not implemented / a PR. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. A-parser Area: The parsing of Rust source code to an AST labels Sep 6, 2020
@digama0
Copy link
Contributor

digama0 commented Sep 7, 2020

By the way, the second error about expected `:` is no longer present on nightly.

@joshtriplett
Copy link
Member

What are the chances we can also provide a mechanically applicable suggestion here that adds an impl block?

@estebank
Copy link
Contributor

estebank commented Sep 8, 2020

What are the chances we can also provide a mechanically applicable suggestion here that adds an impl block?

For a while I've been thinking about writing a generic "item muncher" to be used as a fall-through parser to have a very rudimentary check for "item-like" blocks.

If we had something like that, we could get a reasonable span for the incorrectly nested item, to recover the parse state for the struct (as we already do, we just ignore everything after the parse error other than nesting braces, instead of trying to identify what we've found) and get an appropriate span for an inherent impl block.

I was sure there was a ticket for this already, but I cannot find it :)

By the way, the second error about expected : is no longer present on nightly.

#75513

@LeSeulArtichaut
Copy link
Contributor

I’d like to try to do the first step of changing the current diagnostic

@LeSeulArtichaut LeSeulArtichaut self-assigned this Sep 13, 2020
@LeSeulArtichaut LeSeulArtichaut added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 17, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 7, 2021
…struct, r=jackh726

Improve diagnostics for functions in `struct` definitions

Tries to implement rust-lang#76421.
This is probably going to need unit tests, but I wanted to hear from review all the cases tests should cover.

I'd like to follow up with the "mechanically applicable suggestion here that adds an impl block" step, but I'd need guidance. My idea for now would be to try to parse a function, and if that succeeds, create a dummy `ast::Item` impl block to then format it using `pprust`. Would that be a viable approach? Is there a better alternative?

r? `@matklad` cc `@estebank`
@LeSeulArtichaut LeSeulArtichaut removed their assignment May 19, 2021
@estebank
Copy link
Contributor

estebank commented Aug 4, 2023

Current output:

error: functions are not allowed in struct definitions
 --> src/lib.rs:4:5
  |
1 |   struct S {
  |          - while parsing this struct
...
4 | /     fn do_something() {
5 | |     }
  | |_____^
  |
  = 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

I think we can close this ticket. We also detect when someone uses a let binding, but we don't for consts or types, but I think that's fine.

@estebank estebank closed this as completed Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST C-feature-request Category: A feature request, i.e: not implemented / a PR. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants