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

Identify missing item category in impls #40815

Merged
merged 3 commits into from
Apr 6, 2017
Merged

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 25, 2017

struct S;
impl S {
    pub hello_method(&self) {
        println!("Hello");
    }
}
fn main() { S.hello_method(); }
error: missing `fn` for method declaration
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |        ^ missing `fn`

Fix #40006. r? @pnkfelix CC @jonathandturner @GuillaumeGomez

```rust
struct S;
impl S {
    pub hello_method(&self) {
        println!("Hello");
    }
}
fn main() { S.hello_method(); }
```

```rust
error: can't qualify macro invocation with `pub`
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |     ^^^-            - expected `!` here for a macro invocation
  |        |
  |        did you mean to write `fn` here for a method declaration?
  |
  = help: try adjusting the macro to put `pub` inside the invocation
```
@GuillaumeGomez
Copy link
Member

👍

@sophiajt
Copy link
Contributor

This seems like a step in the right direction, though the help it's giving seems confusing. Is there any way for us to detect if hello_method is a macro? It seems at a glance that these macro messages are distracting the user from the key issue.

@estebank
Copy link
Contributor Author

@jonathandturner I'll see if I can find a macro with the name and output the current output, otherwise output:

error: <ERROR>
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |        ^ missing `fn` here for a method declaration?

What do you think the error message should be for this?

@sophiajt
Copy link
Contributor

@estebank - yeah, if we can get it down to that, that'd be great.

@estebank
Copy link
Contributor Author

@jonathandturner updated output to

error: missing `fn` for method declaration
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |        ^ missing `fn`

It doesn't check if the parsed path is a macro name as I couldn't see a way to check the context in the parser, so I just look ahead to see if a ! is missing and assume it was meant to be a method.

@sophiajt
Copy link
Contributor

Hmm, that sounds best as a separate PR from this one.

@estebank
Copy link
Contributor Author

estebank commented Apr 2, 2017

Ping.

@GuillaumeGomez
Copy link
Member

So for me it's fine as is, so I'll r+. Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 2, 2017

📌 Commit c963d61 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Apr 2, 2017

⌛ Testing commit c963d61 with merge c84f7d9...

@bors
Copy link
Contributor

bors commented Apr 2, 2017

💔 Test failed - status-travis

@estebank
Copy link
Contributor Author

estebank commented Apr 3, 2017

@bors r=GuillaumeGomez

rebased to master

@bors
Copy link
Contributor

bors commented Apr 3, 2017

📌 Commit 8f31e19 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Apr 3, 2017

⌛ Testing commit 8f31e19 with merge 854dca3...

@bors
Copy link
Contributor

bors commented Apr 3, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

alexcrichton commented Apr 3, 2017 via email

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 5, 2017
Identify missing item category in `impl`s

```rust
struct S;
impl S {
    pub hello_method(&self) {
        println!("Hello");
    }
}
fn main() { S.hello_method(); }
```

```rust
error: missing `fn` for method declaration
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |        ^ missing `fn`
```

Fix rust-lang#40006. r? @pnkfelix CC @jonathandturner @GuillaumeGomez
arielb1 pushed a commit to arielb1/rust that referenced this pull request Apr 5, 2017
Identify missing item category in `impl`s

```rust
struct S;
impl S {
    pub hello_method(&self) {
        println!("Hello");
    }
}
fn main() { S.hello_method(); }
```

```rust
error: missing `fn` for method declaration
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |        ^ missing `fn`
```

Fix rust-lang#40006. r? @pnkfelix CC @jonathandturner @GuillaumeGomez
bors added a commit that referenced this pull request Apr 6, 2017
Rollup of 12 pull requests

- Successful merges: #40479, #40561, #40709, #40815, #40909, #40927, #40943, #41015, #41028, #41052, #41054, #41065
- Failed merges:
@bors bors merged commit 8f31e19 into rust-lang:master Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants