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

new_ret_no_self: allow Self in inner type for impl Trait return types #4365

Merged
merged 5 commits into from
Aug 12, 2019

Conversation

lukas-code
Copy link

@lukas-code lukas-code commented Aug 9, 2019

Check the inner types of associated types of a trait when checking for Self in the return type of a new method. This means that the following will no longer warn:

trait Trait {
    type Inner;
}

struct S;

impl S {
    fn new() -> impl Trait<Inner = Option<Self>> {
        struct TraitImpl;

        impl Trait for TraitImpl {
            type Inner = Option<S>;
        }

        TraitImpl
    }
}
#![feature(async_await)]

struct Connection;

impl Connection {
    async fn new() -> Result<Self, ()> {
        Ok(S)
    }
}

closes #4359

changelog: fix new_ret_no_self lint for async new functions.

struct AsyncNew;

impl AsyncNew {
fn new() -> impl Future<Output = Option<Self>> {
Copy link
Member

Choose a reason for hiding this comment

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

Why not just make this function async and use the async_await feature in this test?

Everything else LGTM.

Copy link
Author

Choose a reason for hiding this comment

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

Because the tests seem to use Rust 2015 (which does not have async fns) and I don't know how to change that.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this again 😄

You have to add

// compile-flags: --edition 2018
#![feature(async_await)]

at the top of the test case. We should document this.

@@ -23,10 +25,13 @@ use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
use std::collections::VecDeque;
use std::future::Future;
Copy link
Member

Choose a reason for hiding this comment

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

These imports are now unused.

@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 11, 2019

📌 Commit d553158 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Aug 11, 2019

⌛ Testing commit d553158 with merge d7e4b50...

bors added a commit that referenced this pull request Aug 11, 2019
new_ret_no_self: allow Self in inner type for impl Trait return types

Check the inner types of associated types of a trait when checking for Self in the return type of a `new` method. This means that the following will no longer warn:
```rust
trait Trait {
    type Inner;
}

struct S;

impl S {
    fn new() -> impl Trait<Inner = Option<Self>> {
        struct TraitImpl;

        impl Trait for TraitImpl {
            type Inner = Option<S>;
        }

        TraitImpl
    }
}
```
```rust
#![feature(async_await)]

struct Connection;

impl Connection {
    async fn new() -> Result<Self, ()> {
        Ok(S)
    }
}
```
closes #4359
@bors
Copy link
Collaborator

bors commented Aug 11, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

Forgot the changelog

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 11, 2019

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: Fixed repeated word #4370

@bors
Copy link
Collaborator

bors commented Aug 11, 2019

📌 Commit d553158 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Aug 11, 2019

⌛ Testing commit d553158 with merge dee6217...

bors added a commit that referenced this pull request Aug 11, 2019
new_ret_no_self: allow Self in inner type for impl Trait return types

Check the inner types of associated types of a trait when checking for Self in the return type of a `new` method. This means that the following will no longer warn:
```rust
trait Trait {
    type Inner;
}

struct S;

impl S {
    fn new() -> impl Trait<Inner = Option<Self>> {
        struct TraitImpl;

        impl Trait for TraitImpl {
            type Inner = Option<S>;
        }

        TraitImpl
    }
}
```
```rust
#![feature(async_await)]

struct Connection;

impl Connection {
    async fn new() -> Result<Self, ()> {
        Ok(S)
    }
}
```
closes #4359

changelog: fix `new_ret_no_self` lint for async `new` functions.
@bors
Copy link
Collaborator

bors commented Aug 11, 2019

💔 Test failed - checks-travis

bors added a commit that referenced this pull request Aug 12, 2019
@phansch
Copy link
Member

phansch commented Aug 12, 2019

@bors retry

@bors
Copy link
Collaborator

bors commented Aug 12, 2019

⌛ Testing commit d553158 with merge e255f36...

bors added a commit that referenced this pull request Aug 12, 2019
new_ret_no_self: allow Self in inner type for impl Trait return types

Check the inner types of associated types of a trait when checking for Self in the return type of a `new` method. This means that the following will no longer warn:
```rust
trait Trait {
    type Inner;
}

struct S;

impl S {
    fn new() -> impl Trait<Inner = Option<Self>> {
        struct TraitImpl;

        impl Trait for TraitImpl {
            type Inner = Option<S>;
        }

        TraitImpl
    }
}
```
```rust
#![feature(async_await)]

struct Connection;

impl Connection {
    async fn new() -> Result<Self, ()> {
        Ok(S)
    }
}
```
closes #4359

changelog: fix `new_ret_no_self` lint for async `new` functions.
@bors
Copy link
Collaborator

bors commented Aug 12, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing e255f36 to master...

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.

False positive with new_ret_no_self if new is an async fn that returns a wrapper around Self.
4 participants